[ANNOUNCE] igmpproxy 0.2.1
Hello, new version 0.2.1 of igmpproxy was released which fixes compilation on FreeBSD systems and also fixes broken option -n. It is just a minor version to address these problems. https://github.com/pali/igmpproxy/releases/tag/0.2.1 -- Pali Rohár pali.ro...@gmail.com signature.asc Description: PGP signature
Re: [PATCH v2 0/6] wl1251: Fix MAC address for Nokia N900
On Friday 05 January 2018 02:45:10 Luis R. Rodriguez wrote: > On Tue, Jan 02, 2018 at 08:23:45PM +0100, Pali Rohár wrote: > > On Friday 10 November 2017 00:38:22 Pali Rohár wrote: > > > This patch series fix processing MAC address for wl1251 chip found in > > > Nokia N900. > > > > > > Changes since v1: > > > * Added Acked-by for Pavel Machek > > > * Fixed grammar > > > * Magic numbers for NVS offsets are replaced by defines > > > * Check for validity of mac address NVS data is moved into function > > > * Changed order of patches as Pavel requested > > > > > > Pali Rohár (6): > > > wl1251: Update wl->nvs_len after wl->nvs is valid > > > wl1251: Generate random MAC address only if driver does not have > > > valid > > > wl1251: Parse and use MAC address from supplied NVS data > > > wl1251: Set generated MAC address back to NVS data > > > firmware: Add request_firmware_prefer_user() function > > > wl1251: Use request_firmware_prefer_user() for loading NVS > > > calibration data > > > > > > drivers/base/firmware_class.c | 45 +- > > > drivers/net/wireless/ti/wl1251/Kconfig |1 + > > > drivers/net/wireless/ti/wl1251/main.c | 104 > > > ++-- > > > include/linux/firmware.h |9 +++ > > > 4 files changed, 138 insertions(+), 21 deletions(-) > > > > Hi! Are there any comments for first 4 patches? If not, could they be > > accepted and merged? > > Since the first 4 patches do not touch the firmware API they seem fine to me > so > long as the maintainer accepts them. Maybe resend and clarify you have dropped > the other ones and amend with the new tags. According to get_maintainer.pl, Kalle Valo is maintainer. Kalle Valo, if you do not have any other comments, can you accept first 4 patches? Or do you really need to resent first 4 patches again? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: PGP signature
Re: [PATCH v2 0/6] wl1251: Fix MAC address for Nokia N900
On Friday 10 November 2017 00:38:22 Pali Rohár wrote: > This patch series fix processing MAC address for wl1251 chip found in Nokia > N900. > > Changes since v1: > * Added Acked-by for Pavel Machek > * Fixed grammar > * Magic numbers for NVS offsets are replaced by defines > * Check for validity of mac address NVS data is moved into function > * Changed order of patches as Pavel requested > > Pali Rohár (6): > wl1251: Update wl->nvs_len after wl->nvs is valid > wl1251: Generate random MAC address only if driver does not have > valid > wl1251: Parse and use MAC address from supplied NVS data > wl1251: Set generated MAC address back to NVS data > firmware: Add request_firmware_prefer_user() function > wl1251: Use request_firmware_prefer_user() for loading NVS > calibration data > > drivers/base/firmware_class.c | 45 +- > drivers/net/wireless/ti/wl1251/Kconfig |1 + > drivers/net/wireless/ti/wl1251/main.c | 104 > ++-- > include/linux/firmware.h |9 +++ > 4 files changed, 138 insertions(+), 21 deletions(-) Hi! Are there any comments for first 4 patches? If not, could they be accepted and merged? -- Pali Rohár pali.ro...@gmail.com
[ANNOUNCE] igmpproxy 0.2
Hello, I released a new version 0.2 of the igmpproxy project. Tarball is available at: https://github.com/pali/igmpproxy/releases/tag/0.2 The major change is license change from proprietary Stanford to free compatible with GPLv2+ (mix of 3-clause BSD and GPLv2+). IGMPproxy is a simple dynamic Multicast Routing Daemon using only IGMP signalling. It's intended for simple forwarding of Multicast traffic between networks. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH v2 5/6] firmware: Add request_firmware_prefer_user() function
On Friday 10 November 2017 21:26:01 Luis R. Rodriguez wrote:
> On Fri, Nov 10, 2017 at 12:38:27AM +0100, Pali Rohár wrote:
> > This function works pretty much like request_firmware(), but it prefer
> > usermode helper. If usermode helper fails then it fallback to direct
> > access. Useful for dynamic or model specific firmware data.
> >
> > Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> > ---
> > drivers/base/firmware_class.c | 45
> > +++--
> > include/linux/firmware.h |9 +
> > 2 files changed, 52 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> > index 4b57cf5..c3a9fe5 100644
> > --- a/drivers/base/firmware_class.c
> > +++ b/drivers/base/firmware_class.c
> > @@ -195,6 +195,11 @@ static int __fw_state_check(struct fw_state *fw_st,
> > enum fw_status status)
> > #endif
> > #define FW_OPT_NO_WARN (1U << 3)
> > #define FW_OPT_NOCACHE (1U << 4)
> > +#ifdef CONFIG_FW_LOADER_USER_HELPER
> > +#define FW_OPT_PREFER_USER (1U << 5)
> > +#else
> > +#define FW_OPT_PREFER_USER 0
> > +#endif
>
> I've been cleaning these up these flags [0], which I'll shortly respin based
> on feedback, so this sort of stuff should be avoided at all costs.
>
> Regardless of this even if you *leave* the flag in place and a driver required
> this, but the kernel was compiled without CONFIG_FW_LOADER_USER_HELPER then
> calling fw_load_from_user_helper would just already return -ENOENT, as such it
> would in turn fallback to direct fs loading so the #ifef'ery seems to be not
> needed.
>
> [0] https://lkml.kernel.org/r/20170914225422.31034-1-mcg...@kernel.org
>
> > struct firmware_cache {
> > /* firmware_buf instance will be added into the below list */
> > @@ -1214,13 +1219,26 @@ static void fw_abort_batch_reqs(struct firmware *fw)
> > if (ret <= 0) /* error or already assigned */
> > goto out;
> >
> > - ret = fw_get_filesystem_firmware(device, fw->priv);
> > + if (opt_flags & FW_OPT_PREFER_USER) {
> > + ret = fw_load_from_user_helper(fw, name, device, opt_flags,
> > timeout);
> > + if (ret && !(opt_flags & FW_OPT_NO_WARN)) {
> > + dev_warn(device,
> > +"User helper firmware load for %s failed with
> > error %d\n",
> > +name, ret);
> > + dev_warn(device, "Falling back to direct firmware
> > load\n");
>
> As I had noted before, the usermode helper was really not well designed,
> as such extending further use of it is something we should shy away unless we
> determine its completely necessary.
>
> So what's wrong with this driver failing at direct access, which should be
> fast,
> and relying on a uevent to then work using the current fallback mechanisms?
>
> The commit log in no way documents any of the justifications for further
> extending use of the usermode helper.
Hi! See patch 6/6. It is needed to avoid direct access and wl1251 on
Nokia N900 needs to use userspace helper which prepares firmware data.
Direct access is just fallback when userspace helper is not available.
Without userspace helper on devices where wl1251 do not have own eeprom
memory, wl1251 cannot work.
I know that usermode helper is not well designed, but it is the best
option what we can do for wl1251.
--
Pali Rohár
pali.ro...@gmail.com
[PATCH v2 0/6] wl1251: Fix MAC address for Nokia N900
This patch series fix processing MAC address for wl1251 chip found in Nokia N900. Changes since v1: * Added Acked-by for Pavel Machek * Fixed grammar * Magic numbers for NVS offsets are replaced by defines * Check for validity of mac address NVS data is moved into function * Changed order of patches as Pavel requested Pali Rohár (6): wl1251: Update wl->nvs_len after wl->nvs is valid wl1251: Generate random MAC address only if driver does not have valid wl1251: Parse and use MAC address from supplied NVS data wl1251: Set generated MAC address back to NVS data firmware: Add request_firmware_prefer_user() function wl1251: Use request_firmware_prefer_user() for loading NVS calibration data drivers/base/firmware_class.c | 45 +- drivers/net/wireless/ti/wl1251/Kconfig |1 + drivers/net/wireless/ti/wl1251/main.c | 104 ++-- include/linux/firmware.h |9 +++ 4 files changed, 138 insertions(+), 21 deletions(-) -- 1.7.9.5
[PATCH v2 2/6] wl1251: Generate random MAC address only if driver does not have valid
Before this patch, driver generated random MAC address every time it was
initialized. After that random MAC address could be overwritten with fixed
one, if provided.
This patch changes order. First it tries to read fixed MAC address and if
it fails then driver generates random MAC address.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
Acked-by: Pavel Machek <pa...@ucw.cz>
---
drivers/net/wireless/ti/wl1251/main.c | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/main.c
b/drivers/net/wireless/ti/wl1251/main.c
index 8929bb3..9106c20 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -1492,7 +1492,24 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
wl->hw->queues = 4;
if (wl->use_eeprom)
- wl1251_read_eeprom_mac(wl);
+ ret = wl1251_read_eeprom_mac(wl);
+ else
+ ret = -EINVAL;
+
+ if (ret == 0 && !is_valid_ether_addr(wl->mac_addr))
+ ret = -EINVAL;
+
+ if (ret < 0) {
+ /*
+* In case our MAC address is not correctly set,
+* we use a random but Nokia MAC.
+*/
+ static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
+ memcpy(wl->mac_addr, nokia_oui, 3);
+ get_random_bytes(wl->mac_addr + 3, 3);
+ wl1251_warning("MAC address in eeprom or nvs data is not
valid");
+ wl1251_warning("Setting random MAC address: %pM", wl->mac_addr);
+ }
ret = wl1251_register_hw(wl);
if (ret)
@@ -1513,7 +1530,6 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
struct ieee80211_hw *hw;
struct wl1251 *wl;
int i;
- static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
hw = ieee80211_alloc_hw(sizeof(*wl), _ops);
if (!hw) {
@@ -1563,13 +1579,6 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
INIT_WORK(>irq_work, wl1251_irq_work);
INIT_WORK(>tx_work, wl1251_tx_work);
- /*
-* In case our MAC address is not correctly set,
-* we use a random but Nokia MAC.
-*/
- memcpy(wl->mac_addr, nokia_oui, 3);
- get_random_bytes(wl->mac_addr + 3, 3);
-
wl->state = WL1251_STATE_OFF;
mutex_init(>mutex);
spin_lock_init(>wl_lock);
--
1.7.9.5
[PATCH v2 1/6] wl1251: Update wl->nvs_len after wl->nvs is valid
If kmemdup fails, then wl->nvs_len will contain invalid non-zero size.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
Acked-by: Pavel Machek <pa...@ucw.cz>
---
drivers/net/wireless/ti/wl1251/main.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/main.c
b/drivers/net/wireless/ti/wl1251/main.c
index 9915d83..8929bb3 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -122,8 +122,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
goto out;
}
- wl->nvs_len = fw->size;
- wl->nvs = kmemdup(fw->data, wl->nvs_len, GFP_KERNEL);
+ wl->nvs = kmemdup(fw->data, fw->size, GFP_KERNEL);
if (!wl->nvs) {
wl1251_error("could not allocate memory for the nvs file");
@@ -131,6 +130,8 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
goto out;
}
+ wl->nvs_len = fw->size;
+
ret = 0;
out:
--
1.7.9.5
[PATCH v2 5/6] firmware: Add request_firmware_prefer_user() function
This function works pretty much like request_firmware(), but it prefer
usermode helper. If usermode helper fails then it fallback to direct
access. Useful for dynamic or model specific firmware data.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/base/firmware_class.c | 45 +++--
include/linux/firmware.h |9 +
2 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 4b57cf5..c3a9fe5 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -195,6 +195,11 @@ static int __fw_state_check(struct fw_state *fw_st, enum
fw_status status)
#endif
#define FW_OPT_NO_WARN (1U << 3)
#define FW_OPT_NOCACHE (1U << 4)
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+#define FW_OPT_PREFER_USER (1U << 5)
+#else
+#define FW_OPT_PREFER_USER 0
+#endif
struct firmware_cache {
/* firmware_buf instance will be added into the below list */
@@ -1214,13 +1219,26 @@ static void fw_abort_batch_reqs(struct firmware *fw)
if (ret <= 0) /* error or already assigned */
goto out;
- ret = fw_get_filesystem_firmware(device, fw->priv);
+ if (opt_flags & FW_OPT_PREFER_USER) {
+ ret = fw_load_from_user_helper(fw, name, device, opt_flags,
timeout);
+ if (ret && !(opt_flags & FW_OPT_NO_WARN)) {
+ dev_warn(device,
+"User helper firmware load for %s failed with
error %d\n",
+name, ret);
+ dev_warn(device, "Falling back to direct firmware
load\n");
+ }
+ } else {
+ ret = -EINVAL;
+ }
+
+ if (ret)
+ ret = fw_get_filesystem_firmware(device, fw->priv);
if (ret) {
if (!(opt_flags & FW_OPT_NO_WARN))
dev_warn(device,
"Direct firmware load for %s failed with error
%d\n",
name, ret);
- if (opt_flags & FW_OPT_USERHELPER) {
+ if ((opt_flags & FW_OPT_USERHELPER) && !(opt_flags &
FW_OPT_PREFER_USER)) {
dev_warn(device, "Falling back to user helper\n");
ret = fw_load_from_user_helper(fw, name, device,
opt_flags);
@@ -1329,6 +1347,29 @@ int request_firmware_direct(const struct firmware
**firmware_p,
EXPORT_SYMBOL(request_firmware_into_buf);
/**
+ * request_firmware_prefer_user: - prefer usermode helper for loading firmware
+ * @firmware_p: pointer to firmware image
+ * @name: name of firmware file
+ * @device: device for which firmware is being loaded
+ *
+ * This function works pretty much like request_firmware(), but it prefer
+ * usermode helper. If usermode helper fails then it fallback to direct access.
+ * Useful for dynamic or model specific firmware data.
+ **/
+int request_firmware_prefer_user(const struct firmware **firmware_p,
+ const char *name, struct device *device)
+{
+ int ret;
+
+ __module_get(THIS_MODULE);
+ ret = _request_firmware(firmware_p, name, device, NULL, 0,
+ FW_OPT_UEVENT | FW_OPT_PREFER_USER);
+ module_put(THIS_MODULE);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(request_firmware_prefer_user);
+
+/**
* release_firmware: - release the resource associated with a firmware image
* @fw: firmware resource to release
**/
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index d450808..8584528 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -48,6 +48,8 @@ int request_firmware_nowait(
void (*cont)(const struct firmware *fw, void *context));
int request_firmware_direct(const struct firmware **fw, const char *name,
struct device *device);
+int request_firmware_prefer_user(const struct firmware **fw, const char *name,
+struct device *device);
int request_firmware_into_buf(const struct firmware **firmware_p,
const char *name, struct device *device, void *buf, size_t size);
@@ -78,6 +80,13 @@ static inline int request_firmware_direct(const struct
firmware **fw,
return -EINVAL;
}
+static inline int request_firmware_prefer_user(const struct firmware **fw,
+ const char *name,
+ struct device *device)
+{
+ return -EINVAL;
+}
+
static inline int request_firmware_into_buf(const struct firmware **firmware_p,
const char *name, struct device *device, void *buf, size_t size)
{
--
1.7.9.5
[PATCH v2 4/6] wl1251: Set generated MAC address back to NVS data
In case there is no valid MAC address kernel generates random one. This
patch propagate this generated MAC address back to NVS data which will be
uploaded to wl1251 chip. So HW would have same MAC address as linux kernel
uses.
This should not change any functionality, but it is better to tell wl1251
correct mac address since beginning of chip usage.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/net/wireless/ti/wl1251/main.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/drivers/net/wireless/ti/wl1251/main.c
b/drivers/net/wireless/ti/wl1251/main.c
index d497ba5..1f423be 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -1481,6 +1481,21 @@ static int wl1251_read_nvs_mac(struct wl1251 *wl)
return 0;
}
+static int wl1251_write_nvs_mac(struct wl1251 *wl)
+{
+ int i, ret;
+
+ ret = wl1251_check_nvs_mac(wl);
+ if (ret)
+ return ret;
+
+ /* MAC is stored in reverse order */
+ for (i = 0; i < ETH_ALEN; i++)
+ wl->nvs[NVS_OFF_MAC_DATA + i] = wl->mac_addr[ETH_ALEN - i - 1];
+
+ return 0;
+}
+
static int wl1251_register_hw(struct wl1251 *wl)
{
int ret;
@@ -1546,6 +1561,8 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
memcpy(wl->mac_addr, nokia_oui, 3);
get_random_bytes(wl->mac_addr + 3, 3);
+ if (!wl->use_eeprom)
+ wl1251_write_nvs_mac(wl);
wl1251_warning("MAC address in eeprom or nvs data is not
valid");
wl1251_warning("Setting random MAC address: %pM", wl->mac_addr);
}
--
1.7.9.5
[PATCH v2 3/6] wl1251: Parse and use MAC address from supplied NVS data
This patch implements parsing MAC address from NVS data which are sent to
wl1251 chip. Calibration NVS data could contain valid MAC address and it
will be used instead of randomly generated one.
This patch also moves code for requesting NVS data from userspace to driver
initialization code to make sure that NVS data will be there at time when
permanent MAC address is needed.
Calibration NVS data for wl1251 are device specific. Every device with
wl1251 chip should have been calibrated in factory and needs to provide own
calibration data.
Default example file wl1251-nvs.bin, found in linux-firmware repository,
contains MAC address 00:00:20:07:03:09. So this MAC address is marked as
invalid as it is not real device specific address, just example one.
Format of calibration NVS data can be found at:
http://notaz.gp2x.de/misc/pnd/wl1251/nvs_map.txt
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/net/wireless/ti/wl1251/main.c | 55 -
1 file changed, 47 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/main.c
b/drivers/net/wireless/ti/wl1251/main.c
index 9106c20..d497ba5 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -203,13 +203,6 @@ static int wl1251_chip_wakeup(struct wl1251 *wl)
goto out;
}
- if (wl->nvs == NULL && !wl->use_eeprom) {
- /* No NVS from netlink, try to get it from the filesystem */
- ret = wl1251_fetch_nvs(wl);
- if (ret < 0)
- goto out;
- }
-
out:
return ret;
}
@@ -1448,6 +1441,46 @@ static int wl1251_read_eeprom_mac(struct wl1251 *wl)
return 0;
}
+#define NVS_OFF_MAC_LEN 0x19
+#define NVS_OFF_MAC_ADDR_LO 0x1a
+#define NVS_OFF_MAC_ADDR_HI 0x1b
+#define NVS_OFF_MAC_DATA 0x1c
+
+static int wl1251_check_nvs_mac(struct wl1251 *wl)
+{
+ if (wl->nvs_len < 0x24)
+ return -ENODATA;
+
+ /* length is 2 and data address is 0x546c (ANDed with 0xfffe) */
+ if (wl->nvs[NVS_OFF_MAC_LEN] != 2 ||
+ wl->nvs[NVS_OFF_MAC_ADDR_LO] != 0x6d ||
+ wl->nvs[NVS_OFF_MAC_ADDR_HI] != 0x54)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int wl1251_read_nvs_mac(struct wl1251 *wl)
+{
+ u8 mac[ETH_ALEN];
+ int i, ret;
+
+ ret = wl1251_check_nvs_mac(wl);
+ if (ret)
+ return ret;
+
+ /* MAC is stored in reverse order */
+ for (i = 0; i < ETH_ALEN; i++)
+ mac[i] = wl->nvs[NVS_OFF_MAC_DATA + ETH_ALEN - i - 1];
+
+ /* 00:00:20:07:03:09 is in example file wl1251-nvs.bin, so invalid */
+ if (ether_addr_equal_unaligned(mac, "\x00\x00\x20\x07\x03\x09"))
+ return -EINVAL;
+
+ memcpy(wl->mac_addr, mac, ETH_ALEN);
+ return 0;
+}
+
static int wl1251_register_hw(struct wl1251 *wl)
{
int ret;
@@ -1491,10 +1524,16 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
wl->hw->queues = 4;
+ if (wl->nvs == NULL && !wl->use_eeprom) {
+ ret = wl1251_fetch_nvs(wl);
+ if (ret < 0)
+ goto out;
+ }
+
if (wl->use_eeprom)
ret = wl1251_read_eeprom_mac(wl);
else
- ret = -EINVAL;
+ ret = wl1251_read_nvs_mac(wl);
if (ret == 0 && !is_valid_ether_addr(wl->mac_addr))
ret = -EINVAL;
--
1.7.9.5
[PATCH v2 6/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
NVS calibration data for wl1251 are model specific. Every one device with
wl1251 chip has different and calibrated in factory.
Not all wl1251 chips have own EEPROM where are calibration data stored. And
in that case there is no "standard" place. Every device has stored them on
different place (some in rootfs file, some in dedicated nand partition,
some in another proprietary structure).
Kernel wl1251 driver cannot support every one different storage decided by
device manufacture so it will use request_firmware_prefer_user() call for
loading NVS calibration data and userspace helper will be responsible to
prepare correct data.
In case userspace helper fails request_firmware_prefer_user() still try to
load data file directly from VFS as fallback mechanism.
On Nokia N900 device, which has wl1251 chip, NVS calibration data are
stored in CAL nand partition. CAL is proprietary Nokia key/value format for
nand devices.
With this patch it is finally possible to load correct model specific NVS
calibration data for Nokia N900.
Userspace tool for reading NVS calibration data on Nokia N900 is available
in git repository at: https://github.com/community-ssu/wl1251-cal
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/net/wireless/ti/wl1251/Kconfig |1 +
drivers/net/wireless/ti/wl1251/main.c |2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ti/wl1251/Kconfig
b/drivers/net/wireless/ti/wl1251/Kconfig
index 7142ccf..affe154 100644
--- a/drivers/net/wireless/ti/wl1251/Kconfig
+++ b/drivers/net/wireless/ti/wl1251/Kconfig
@@ -2,6 +2,7 @@ config WL1251
tristate "TI wl1251 driver support"
depends on MAC80211
select FW_LOADER
+ select FW_LOADER_USER_HELPER
select CRC7
---help---
This will enable TI wl1251 driver support. The drivers make
diff --git a/drivers/net/wireless/ti/wl1251/main.c
b/drivers/net/wireless/ti/wl1251/main.c
index 1f423be..e9d232c 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -108,7 +108,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
struct device *dev = wiphy_dev(wl->hw->wiphy);
int ret;
- ret = request_firmware(, WL1251_NVS_NAME, dev);
+ ret = request_firmware_prefer_user(, WL1251_NVS_NAME, dev);
if (ret < 0) {
wl1251_error("could not get nvs file: %d", ret);
--
1.7.9.5
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Wednesday 17 May 2017 15:04:50 Johannes Berg wrote: > On Wed, 2017-05-17 at 14:53 +0200, Pali Rohár wrote: > > > > In fact, why should the *driver* care either? IOW - why should > > > "request_firmware_prefer_user()" even exist? > > > > There are default/example NVS data, which are stored in /lib/firmware > > and installed by linux-firmware package. > [...] > > Oh, so you're saying you want this to invert the order ... Ok, that > makes some sense. Yes! I thought that this fact can be understood from commit message. If not, I can change it, but provide how to improve it. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Wednesday 17 May 2017 14:06:06 Johannes Berg wrote: > On Tue, 2017-05-16 at 01:13 +0200, Luis R. Rodriguez wrote: > > > > Now for N900 case there is a similar scenario > > > > alhtough it has additional requirement to go to user-space due to > > > > need to use a proprietary library to obtain the NVS calibration > > > > data. My thought: Why should firmware_class care? > > > > Agreed. > > In fact, why should the *driver* care either? IOW - why should > "request_firmware_prefer_user()" even exist? There are default/example NVS data, which are stored in /lib/firmware and installed by linux-firmware package. Those example calibration data should not be used for real usage, but Pavel told us that on N900 they are enough for working WIFI connection. They does not contain valid MAC address, so kernel should generate some (random?). So kernel driver should get NVS calibration data from userspace (which know how where to get or how to prepare them) and in case userspace do not have it, then we can try fallback to those example data (as people reported us they can be useful instead of non-working WIFI). And that fallback is working by direct firmware loading from kernel. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Tuesday 31 January 2017 07:59:18 Tony Lindgren wrote: > * Kalle Valo <kv...@codeaurora.org> [170130 22:36]: > > Tony Lindgren <t...@atomide.com> writes: > > > > > * Pavel Machek <pa...@ucw.cz> [170127 11:41]: > > >> On Fri 2017-01-27 17:23:07, Kalle Valo wrote: > > >> > Pali Rohár <pali.ro...@gmail.com> writes: > > >> > > > >> > > On Friday 27 January 2017 14:26:22 Kalle Valo wrote: > > >> > >> Pali Rohár <pali.ro...@gmail.com> writes: > > >> > >> > > >> > >> > 2) It was already tested that example NVS data can be used for > > >> > >> > N900 e.g. > > >> > >> > for SSH connection. If real correct data are not available it is > > >> > >> > better > > >> > >> > to use at least those example (and probably log warning message) > > >> > >> > so user > > >> > >> > can connect via SSH and start investigating where is problem. > > >> > >> > > >> > >> I disagree. Allowing default calibration data to be used can be > > >> > >> unnoticed by user and left her wondering why wifi works so badly. > > >> > > > > >> > > So there are only two options: > > >> > > > > >> > > 1) Disallow it and so these users will have non-working wifi. > > >> > > > > >> > > 2) Allow those data to be used as fallback mechanism. > > >> > > > > >> > > And personally I'm against 1) because it will break wifi support for > > >> > > *all* Nokia N900 devices right now. > > >> > > > >> > All two of them? :) > > >> > > >> Umm. You clearly want a flock of angry penguins at your doorsteps :-). > > > > > > Well this silly issue of symlinking and renaming nvs files in a standard > > > Linux distro was also hitting me on various devices with wl12xx/wl18xx > > > trying to use the same rootfs. > > > > > > Why don't we just set a custom compatible property for n900 that then > > > picks up some other nvs file instead of the default? > > > > Please don't. An ugly kernel workaround in kernel because of user space > > problems is a bad idea. wl1251 should just ask for NVS file from user > > space, it shouldn't care if it's a "default" file or something else. > > That's a user space policy decision. > > Grr I keep forgetting it needs to be for each device manufactured so > yeah that won't work. > > The names of standard distro files are hardcoded into the kernel > driver so it's also a kernel problem though :p > > How about a custom devices tree property saying "needs-custom-firmware"? How does it help request_firmware() call which automatically loads firmware file from VFS (if is available)? > Something that would prevent anything being loaded until user space > loads the firmware. It could also be set in the driver automatically > based on the compatible flag if we always want it enabled. And we could > have some cmdline option to ignore it. Or the other way around whatever > makes sense. So you just want to kernel automatically prevent loading firmware file (based on flag which driver can set). That is similar approach as mine. > > Why can't you do something like this: > > > > * rename the NVS file linux-firmware to wl1251-nvs.bin.example > > As that name is hardcoded in the kernel and that file is provided by > all standard distros, let's assume we just have to deal with that ABI > forever. Yes. > > * before distro updates linux-firmware create yours own deb/rpm/whatever > > package "wl1251-firmware" which installs your flavor of nvs file (or > > the user fallback helper if more dynamic functionality is preferred) > > And that won't work when using the same file system on other machines. > > Think NFSroot for example. At least I'm using the same NFSroot across > about 15 different machines including one n900 macro board with smc91x > Ethernet. Exactly problem which we already discussed in previous emails. You cannot serve one file (loaded by direct request_firmware) when your rootfs is readonly, e.g. comes via NFS shared for more devices... -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Monday 30 January 2017 18:53:09 Tony Lindgren wrote: > * Pavel Machek <pa...@ucw.cz> [170127 11:41]: > > On Fri 2017-01-27 17:23:07, Kalle Valo wrote: > > > Pali Rohár <pali.ro...@gmail.com> writes: > > > > On Friday 27 January 2017 14:26:22 Kalle Valo wrote: > > > >> Pali Rohár <pali.ro...@gmail.com> writes: > > > >> > 2) It was already tested that example NVS data can be used > > > >> > for N900 e.g. for SSH connection. If real correct data are > > > >> > not available it is better to use at least those example > > > >> > (and probably log warning message) so user can connect via > > > >> > SSH and start investigating where is problem. > > > >> > > > >> I disagree. Allowing default calibration data to be used can > > > >> be unnoticed by user and left her wondering why wifi works so > > > >> badly. > > > > > > > > So there are only two options: > > > > > > > > 1) Disallow it and so these users will have non-working wifi. > > > > > > > > 2) Allow those data to be used as fallback mechanism. > > > > > > > > And personally I'm against 1) because it will break wifi > > > > support for *all* Nokia N900 devices right now. > > > > > > All two of them? :) > > > > Umm. You clearly want a flock of angry penguins at your doorsteps > > :-). > > Well this silly issue of symlinking and renaming nvs files in a > standard Linux distro was also hitting me on various devices with > wl12xx/wl18xx trying to use the same rootfs. wl12xx/wl18xx have probably exactly same problem as wl1251. > Why don't we just set a custom compatible property for n900 that then > picks up some other nvs file instead of the default? But that still does not solve this problem correctly. Every n900 device have different NVS file. If we allow to load firmware directly from VFS without userspace helper we would see again same problem. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Friday 27 January 2017 17:23:07 Kalle Valo wrote: > Pali Rohár <pali.ro...@gmail.com> writes: > > > On Friday 27 January 2017 14:26:22 Kalle Valo wrote: > >> Pali Rohár <pali.ro...@gmail.com> writes: > >> > >> > 2) It was already tested that example NVS data can be used for N900 e.g. > >> > for SSH connection. If real correct data are not available it is better > >> > to use at least those example (and probably log warning message) so user > >> > can connect via SSH and start investigating where is problem. > >> > >> I disagree. Allowing default calibration data to be used can be > >> unnoticed by user and left her wondering why wifi works so badly. > > > > So there are only two options: > > > > 1) Disallow it and so these users will have non-working wifi. > > > > 2) Allow those data to be used as fallback mechanism. > > > > And personally I'm against 1) because it will break wifi support for > > *all* Nokia N900 devices right now. > > All two of them? :) Ehm... > But not working is exactly my point, if correct calibration data is not > available wifi should not work. And it's not only about functionality > problems, there's also the regulatory aspect. About functionality, Pavel confirmed too that SSH is somehow working... Regulatory aspect is different, but via iw can be manually configured some settings. > >> > 3) If we do rename *now* we will totally break wifi support on Nokia > >> > N900. > >> > >> Then the distro should fix that when updating the linux-firmware > >> packages. Can you provide details about the setup, what distro etc? > > > > Debian stable, Ubuntu LTSs 14.04, 16.04. > > You can run these out of box on N900? Out-of-box I can run Kubuntu 12.04 (which is LTS too). They had prepared special image for N900 and I still have it on uSD card. I guess that new versions of Ubuntu could somehow work (maybe not out-of-box but with some changes) and Pavel has working Debian. Also basic support needed for wifi and SSH server is probably working with any distribution targeting armv7-a or omap3. So yes, I can say it is out-of-box. We will not have GSM calls or camera support, but wifi breakage is there. > > And I think that other LTS distributions contains that example nvs > > file too (I'm not going to verify others, but list will be probably > > long). Upgrading linux-firmware is against policy of those > > distributions. So no this is not an solution. > > So instead we should workaround distro policies in kernel? Come on. > > Seriously, just rename the file in linux-firmware and file a bug (with a > patch) to distros. If they don't fix the bug you just have to do a > custom hack for N900. But such is life. I do not see point what will be changed. I rename that file and after system update (or integrity check) it will be there again. And if I do that, what prevents kernel to stop using NVS file from /lib/firmware/? Nothing, original problem (which is going solved by this patch series) still remains. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Friday 27 January 2017 13:53:28 Arend Van Spriel wrote: > On 27-1-2017 13:26, Kalle Valo wrote: > > Pali Rohár <pali.ro...@gmail.com> writes: > > > >> On Friday 27 January 2017 13:49:03 Kalle Valo wrote: > >>> Pali Rohár <pali.ro...@gmail.com> writes: > >>> > >>>>> So > >>>>> for those other platforms there will be a delay waiting for user-mode > >>>>> helper to fail, before trying to get nvs file from /lib/firmware. > >>>> > >>>> Yes, there will be. But there is no easy way to fix this problem that > >>>> kernel is trying to use default/example NVS data... > >>> > >>> Kernel is doing correctly and requesting NVS data as expected, the > >>> problem here is that linux-firmware claims that the example NVS data is > >>> real calibration data (which it is not). Distros should not use that, > >>> only developers for testing purposes. We should not courage users using > >>> example calibration data. > >>> > >>> The simple fix is to rename the NVS file in linux-firmware to something > >>> like wl1251-nvs.bin.example, no need to workaround this in kernel. If > >>> you send a patch to linux-firmware I'm happy to ack that. > >> > >> I agree with rename and fact that default/example data should not be > >> used. > >> > >> But... > >> > >> 1) Kernel should not read device/model specific data from VFS where > >> are stored not-device-specific files preinstalled by linux > >> distributions. > >> > >> And linux distributions are already putting files into VFS and kernel > >> cannot enforce userspace to not do that (as they are already doing it). > > > > I'm having problems to understand what you are saying here. > > This is a personal opinion. I read it as: /lib/firmware can only contain > files for from linux-firmware. > > At least the device-specific vs. non-device-specific does not seem to > hold. The firmware files that we have in the linux-firmware repository > are very device-specific. Unless you mean the 'platform' when talking > about 'device'. Here I'm talking about files which are specific per unit. Every one N900 has different NVS file (stored in CAL) and so every one N900 device needs its own NVS file. And we cannot store thousands of NVS files into linux-firmware repository for each N900 which was ever produced in factory. Firmware files in linux-firmware repository are "device" specific, but "filename" of that file describe exactly for which "device" it is specific. But there are thousands of different NVS files for one filename "wl1251-nvs.bin" and we cannot use one particular for another device. In linux-firmware is stored "wl1251-nvs.bin" file with example data. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Friday 27 January 2017 14:26:22 Kalle Valo wrote: > Pali Rohár <pali.ro...@gmail.com> writes: > > > On Friday 27 January 2017 13:49:03 Kalle Valo wrote: > >> Pali Rohár <pali.ro...@gmail.com> writes: > >> > >> >> So > >> >> for those other platforms there will be a delay waiting for user-mode > >> >> helper to fail, before trying to get nvs file from /lib/firmware. > >> > > >> > Yes, there will be. But there is no easy way to fix this problem that > >> > kernel is trying to use default/example NVS data... > >> > >> Kernel is doing correctly and requesting NVS data as expected, the > >> problem here is that linux-firmware claims that the example NVS data is > >> real calibration data (which it is not). Distros should not use that, > >> only developers for testing purposes. We should not courage users using > >> example calibration data. > >> > >> The simple fix is to rename the NVS file in linux-firmware to something > >> like wl1251-nvs.bin.example, no need to workaround this in kernel. If > >> you send a patch to linux-firmware I'm happy to ack that. > > > > I agree with rename and fact that default/example data should not be > > used. > > > > But... > > > > 1) Kernel should not read device/model specific data from VFS where > > are stored not-device-specific files preinstalled by linux > > distributions. > > > > And linux distributions are already putting files into VFS and kernel > > cannot enforce userspace to not do that (as they are already doing it). > > I'm having problems to understand what you are saying here. I'm saying that linux distributions are putting files to /lib/firmware which comes from some sources already released. You cannot force linux distributions to stop putting particular file to /lib/firmware *now* after it was already released and recommended. > > 2) It was already tested that example NVS data can be used for N900 e.g. > > for SSH connection. If real correct data are not available it is better > > to use at least those example (and probably log warning message) so user > > can connect via SSH and start investigating where is problem. > > I disagree. Allowing default calibration data to be used can be > unnoticed by user and left her wondering why wifi works so badly. So there are only two options: 1) Disallow it and so these users will have non-working wifi. 2) Allow those data to be used as fallback mechanism. And personally I'm against 1) because it will break wifi support for *all* Nokia N900 devices right now. > > 3) If we do rename *now* we will totally break wifi support on Nokia > > N900. > > Then the distro should fix that when updating the linux-firmware > packages. Can you provide details about the setup, what distro etc? Debian stable, Ubuntu LTSs 14.04, 16.04. And I think that other LTS distributions contains that example nvs file too (I'm not going to verify others, but list will be probably long). Upgrading linux-firmware is against policy of those distributions. So no this is not an solution. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Friday 27 January 2017 13:49:03 Kalle Valo wrote: > Pali Rohár <pali.ro...@gmail.com> writes: > > >> So > >> for those other platforms there will be a delay waiting for user-mode > >> helper to fail, before trying to get nvs file from /lib/firmware. > > > > Yes, there will be. But there is no easy way to fix this problem that > > kernel is trying to use default/example NVS data... > > Kernel is doing correctly and requesting NVS data as expected, the > problem here is that linux-firmware claims that the example NVS data is > real calibration data (which it is not). Distros should not use that, > only developers for testing purposes. We should not courage users using > example calibration data. > > The simple fix is to rename the NVS file in linux-firmware to something > like wl1251-nvs.bin.example, no need to workaround this in kernel. If > you send a patch to linux-firmware I'm happy to ack that. I agree with rename and fact that default/example data should not be used. But... 1) Kernel should not read device/model specific data from VFS where are stored not-device-specific files preinstalled by linux distributions. And linux distributions are already putting files into VFS and kernel cannot enforce userspace to not do that (as they are already doing it). 2) It was already tested that example NVS data can be used for N900 e.g. for SSH connection. If real correct data are not available it is better to use at least those example (and probably log warning message) so user can connect via SSH and start investigating where is problem. 3) If we do rename *now* we will totally break wifi support on Nokia N900. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Friday 27 January 2017 11:19:25 Arend Van Spriel wrote: > On 27-1-2017 11:10, Pali Rohár wrote: > > On Friday 27 January 2017 11:05:32 Arend Van Spriel wrote: > >> On 27-1-2017 10:43, Pali Rohár wrote: > >>> On Friday 27 January 2017 09:33:40 Kalle Valo wrote: > >>>> Pali Rohár <pali.ro...@gmail.com> writes: > >>>> > >>>>> NVS calibration data for wl1251 are model specific. Every one device > >>>>> with > >>>>> wl1251 chip has different and calibrated in factory. > >>>>> > >>>>> Not all wl1251 chips have own EEPROM where are calibration data stored. > >>>>> And > >>>>> in that case there is no "standard" place. Every device has stored them > >>>>> on > >>>>> different place (some in rootfs file, some in dedicated nand partition, > >>>>> some in another proprietary structure). > >>>>> > >>>>> Kernel wl1251 driver cannot support every one different storage decided > >>>>> by > >>>>> device manufacture so it will use request_firmware_prefer_user() call > >>>>> for > >>>>> loading NVS calibration data and userspace helper will be responsible to > >>>>> prepare correct data. > >>>>> > >>>>> In case userspace helper fails request_firmware_prefer_user() still try > >>>>> to > >>>>> load data file directly from VFS as fallback mechanism. > >>>>> > >>>>> On Nokia N900 device which has wl1251 chip, NVS calibration data are > >>>>> stored > >>>>> in CAL nand partition. CAL is proprietary Nokia key/value format for > >>>>> nand > >>>>> devices. > >>>>> > >>>>> With this patch it is finally possible to load correct model specific > >>>>> NVS > >>>>> calibration data for Nokia N900. > >>>>> > >>>>> Signed-off-by: Pali Rohár <pali.ro...@gmail.com> > >>>>> --- > >>>>> drivers/net/wireless/ti/wl1251/Kconfig |1 + > >>>>> drivers/net/wireless/ti/wl1251/main.c |2 +- > >>>>> 2 files changed, 2 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/drivers/net/wireless/ti/wl1251/Kconfig > >>>>> b/drivers/net/wireless/ti/wl1251/Kconfig > >>>>> index 7142ccf..affe154 100644 > >>>>> --- a/drivers/net/wireless/ti/wl1251/Kconfig > >>>>> +++ b/drivers/net/wireless/ti/wl1251/Kconfig > >>>>> @@ -2,6 +2,7 @@ config WL1251 > >>>>> tristate "TI wl1251 driver support" > >>>>> depends on MAC80211 > >>>>> select FW_LOADER > >>>>> + select FW_LOADER_USER_HELPER > >>>>> select CRC7 > >>>>> ---help--- > >>>>> This will enable TI wl1251 driver support. The drivers make > >>>>> diff --git a/drivers/net/wireless/ti/wl1251/main.c > >>>>> b/drivers/net/wireless/ti/wl1251/main.c > >>>>> index 208f062..24f8866 100644 > >>>>> --- a/drivers/net/wireless/ti/wl1251/main.c > >>>>> +++ b/drivers/net/wireless/ti/wl1251/main.c > >>>>> @@ -110,7 +110,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl) > >>>>> struct device *dev = wiphy_dev(wl->hw->wiphy); > >>>>> int ret; > >>>>> > >>>>> - ret = request_firmware(, WL1251_NVS_NAME, dev); > >>>>> + ret = request_firmware_prefer_user(, WL1251_NVS_NAME, dev); > >>>> > >>>> I don't see the need for this. Just remove the default nvs file from > >>>> filesystem and the fallback user helper will be always used, right? > >>> > >>> It is part of linux-firmware repository. And already part of all > >>> previous versions of linux-firmware packages in lot of linux > >>> distributions. So removing it is not possible... > >> > >> You are probably saying that on your platform you can not remove > >> anything from /lib/firmware, right? I don't see how you come from "it is > >> part of firmware package" to "removing is not possible". Trying to > >> understand this and
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Friday 27 January 2017 11:05:32 Arend Van Spriel wrote: > On 27-1-2017 10:43, Pali Rohár wrote: > > On Friday 27 January 2017 09:33:40 Kalle Valo wrote: > >> Pali Rohár <pali.ro...@gmail.com> writes: > >> > >>> NVS calibration data for wl1251 are model specific. Every one device with > >>> wl1251 chip has different and calibrated in factory. > >>> > >>> Not all wl1251 chips have own EEPROM where are calibration data stored. > >>> And > >>> in that case there is no "standard" place. Every device has stored them on > >>> different place (some in rootfs file, some in dedicated nand partition, > >>> some in another proprietary structure). > >>> > >>> Kernel wl1251 driver cannot support every one different storage decided by > >>> device manufacture so it will use request_firmware_prefer_user() call for > >>> loading NVS calibration data and userspace helper will be responsible to > >>> prepare correct data. > >>> > >>> In case userspace helper fails request_firmware_prefer_user() still try to > >>> load data file directly from VFS as fallback mechanism. > >>> > >>> On Nokia N900 device which has wl1251 chip, NVS calibration data are > >>> stored > >>> in CAL nand partition. CAL is proprietary Nokia key/value format for nand > >>> devices. > >>> > >>> With this patch it is finally possible to load correct model specific NVS > >>> calibration data for Nokia N900. > >>> > >>> Signed-off-by: Pali Rohár <pali.ro...@gmail.com> > >>> --- > >>> drivers/net/wireless/ti/wl1251/Kconfig |1 + > >>> drivers/net/wireless/ti/wl1251/main.c |2 +- > >>> 2 files changed, 2 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/net/wireless/ti/wl1251/Kconfig > >>> b/drivers/net/wireless/ti/wl1251/Kconfig > >>> index 7142ccf..affe154 100644 > >>> --- a/drivers/net/wireless/ti/wl1251/Kconfig > >>> +++ b/drivers/net/wireless/ti/wl1251/Kconfig > >>> @@ -2,6 +2,7 @@ config WL1251 > >>> tristate "TI wl1251 driver support" > >>> depends on MAC80211 > >>> select FW_LOADER > >>> + select FW_LOADER_USER_HELPER > >>> select CRC7 > >>> ---help--- > >>> This will enable TI wl1251 driver support. The drivers make > >>> diff --git a/drivers/net/wireless/ti/wl1251/main.c > >>> b/drivers/net/wireless/ti/wl1251/main.c > >>> index 208f062..24f8866 100644 > >>> --- a/drivers/net/wireless/ti/wl1251/main.c > >>> +++ b/drivers/net/wireless/ti/wl1251/main.c > >>> @@ -110,7 +110,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl) > >>> struct device *dev = wiphy_dev(wl->hw->wiphy); > >>> int ret; > >>> > >>> - ret = request_firmware(, WL1251_NVS_NAME, dev); > >>> + ret = request_firmware_prefer_user(, WL1251_NVS_NAME, dev); > >> > >> I don't see the need for this. Just remove the default nvs file from > >> filesystem and the fallback user helper will be always used, right? > > > > It is part of linux-firmware repository. And already part of all > > previous versions of linux-firmware packages in lot of linux > > distributions. So removing it is not possible... > > You are probably saying that on your platform you can not remove > anything from /lib/firmware, right? I don't see how you come from "it is > part of firmware package" to "removing is not possible". Trying to > understand this and it makes no sense. It is already in linux distribution packages. If I remove that file from file system it will be placed there again by package management or it it will throw error message about system integrity (missing file, etc...). Also that file is already in linux-firmware git and so is propagated to /lib/firmware by anybody who is using linux-firmware. > >> Like we discussed earlier, the default nvs file should not be used by > >> normal users. > > > > But already is and we need to deal with this fact. > > Why? Because everybody has already installed it. > Are there other platforms that use the default nvs file and have a > working wifi. I do not know. > So your "removing is not possible" would be about > regression for those? Yes, that is possible. Also you can use wifi on Nokia N900 with this default file. Yes it is not recommended and probably has performance problems... but more people use it for SSH and it is working. Pavel could confirm this. So yes, if you remove that file *now* there is regression for Nokia N900 when you are using SSH over wifi. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data
On Friday 27 January 2017 09:56:09 Kalle Valo wrote: > Pali Rohár <pali.ro...@gmail.com> writes: > > > In case there is no valid MAC address kernel generates random one. This > > patch propagate this generated MAC address back to NVS data which will be > > uploaded to wl1251 chip. So HW would have same MAC address as linux kernel > > uses. > > > > Signed-off-by: Pali Rohár <pali.ro...@gmail.com> > > Why? What issue does this fix? Send permanent MAC address to wl1251 chip, same what is doing wl12xx driver. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Friday 27 January 2017 09:33:40 Kalle Valo wrote: > Pali Rohár <pali.ro...@gmail.com> writes: > > > NVS calibration data for wl1251 are model specific. Every one device with > > wl1251 chip has different and calibrated in factory. > > > > Not all wl1251 chips have own EEPROM where are calibration data stored. And > > in that case there is no "standard" place. Every device has stored them on > > different place (some in rootfs file, some in dedicated nand partition, > > some in another proprietary structure). > > > > Kernel wl1251 driver cannot support every one different storage decided by > > device manufacture so it will use request_firmware_prefer_user() call for > > loading NVS calibration data and userspace helper will be responsible to > > prepare correct data. > > > > In case userspace helper fails request_firmware_prefer_user() still try to > > load data file directly from VFS as fallback mechanism. > > > > On Nokia N900 device which has wl1251 chip, NVS calibration data are stored > > in CAL nand partition. CAL is proprietary Nokia key/value format for nand > > devices. > > > > With this patch it is finally possible to load correct model specific NVS > > calibration data for Nokia N900. > > > > Signed-off-by: Pali Rohár <pali.ro...@gmail.com> > > --- > > drivers/net/wireless/ti/wl1251/Kconfig |1 + > > drivers/net/wireless/ti/wl1251/main.c |2 +- > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/ti/wl1251/Kconfig > > b/drivers/net/wireless/ti/wl1251/Kconfig > > index 7142ccf..affe154 100644 > > --- a/drivers/net/wireless/ti/wl1251/Kconfig > > +++ b/drivers/net/wireless/ti/wl1251/Kconfig > > @@ -2,6 +2,7 @@ config WL1251 > > tristate "TI wl1251 driver support" > > depends on MAC80211 > > select FW_LOADER > > + select FW_LOADER_USER_HELPER > > select CRC7 > > ---help--- > > This will enable TI wl1251 driver support. The drivers make > > diff --git a/drivers/net/wireless/ti/wl1251/main.c > > b/drivers/net/wireless/ti/wl1251/main.c > > index 208f062..24f8866 100644 > > --- a/drivers/net/wireless/ti/wl1251/main.c > > +++ b/drivers/net/wireless/ti/wl1251/main.c > > @@ -110,7 +110,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl) > > struct device *dev = wiphy_dev(wl->hw->wiphy); > > int ret; > > > > - ret = request_firmware(, WL1251_NVS_NAME, dev); > > + ret = request_firmware_prefer_user(, WL1251_NVS_NAME, dev); > > I don't see the need for this. Just remove the default nvs file from > filesystem and the fallback user helper will be always used, right? It is part of linux-firmware repository. And already part of all previous versions of linux-firmware packages in lot of linux distributions. So removing it is not possible... > Like we discussed earlier, the default nvs file should not be used by > normal users. But already is and we need to deal with this fact. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Monday 26 December 2016 16:43:53 Pavel Machek wrote: > Hi! > > > > > NVS calibration data for wl1251 are model specific. Every one > > > > device with wl1251 chip has different and calibrated in > > > > factory. > > > > > > > > Not all wl1251 chips have own EEPROM where are calibration data > > > > stored. And in that case there is no "standard" place. Every > > > > device has stored them on different place (some in rootfs file, > > > > some in dedicated nand partition, some in another proprietary > > > > structure). > > > > > > > > Kernel wl1251 driver cannot support every one different storage > > > > decided by device manufacture so it will use > > > > request_firmware_prefer_user() call for loading NVS calibration > > > > data and userspace helper will be responsible to prepare > > > > correct data. > > > > > > Responding to this patch as it provides a lot of context to > > > discuss. As you might have gathered from earlier discussions I > > > am not a fan of using user-space helper. I can agree that the > > > kernel driver, wl1251 in this case, should be agnostic to > > > platform specific details regarding storage solutions and the > > > firmware api should hide that. However, it seems your only > > > solution is adding user-space to the mix and changing the api > > > towards that. Can we solve it without user-space help? > > > > Without userspace helper it means that userspace helper code must > > be integrated into kernel. > > > > So what is userspace helper doing? > > > > 1) Read MAC address from CAL > > 2) Read NVS data from CAL > > 3) Modify MAC address in memory NVS data (new for this patch > > series) 4) Modify in memory NVS data if we in FCC country > > > > Checking for country is done via dbus call to either Maemo cellular > > daemon or alternatively via REGDOMAIN in /etc/default/crda. I have > > plan to use ofono (instead Maemo cellular daemon) too... > > > > Currently we are using closed Nokia proprietary CAL library. > > > > Steps 1) and 2) needs closed library, step 4) needs dbus call. > > I guess pointer to the source code implementing this would be > welcome. Here is current code: https://github.com/community-ssu/wl1251-cal (there is implemented also Maemo netlink interface) > > > But on other devices that use wl1251, but for instance have no > > > userspace helper the request to userspace will fail (after 60 > > > sec?) and try VFS after that. Maybe not so nice. > > > > Currently support for those devices is broken (like for N900) as > > without proper NVS data they do not work correctly... > > Is it expected to work at all, perhaps with degraded performance / > range? Because it seems to work for me. Yes, some degraded performance or problems with connecting is expected. And random MAC address at every boot. Plus some regulatory problems in FCC countries. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
On Sunday 25 December 2016 21:15:40 Arend Van Spriel wrote: > On 24-12-2016 17:52, Pali Rohár wrote: > > NVS calibration data for wl1251 are model specific. Every one > > device with wl1251 chip has different and calibrated in factory. > > > > Not all wl1251 chips have own EEPROM where are calibration data > > stored. And in that case there is no "standard" place. Every > > device has stored them on different place (some in rootfs file, > > some in dedicated nand partition, some in another proprietary > > structure). > > > > Kernel wl1251 driver cannot support every one different storage > > decided by device manufacture so it will use > > request_firmware_prefer_user() call for loading NVS calibration > > data and userspace helper will be responsible to prepare correct > > data. > > Responding to this patch as it provides a lot of context to discuss. > As you might have gathered from earlier discussions I am not a fan > of using user-space helper. I can agree that the kernel driver, > wl1251 in this case, should be agnostic to platform specific details > regarding storage solutions and the firmware api should hide that. > However, it seems your only solution is adding user-space to the mix > and changing the api towards that. Can we solve it without > user-space help? Without userspace helper it means that userspace helper code must be integrated into kernel. So what is userspace helper doing? 1) Read MAC address from CAL 2) Read NVS data from CAL 3) Modify MAC address in memory NVS data (new for this patch series) 4) Modify in memory NVS data if we in FCC country Checking for country is done via dbus call to either Maemo cellular daemon or alternatively via REGDOMAIN in /etc/default/crda. I have plan to use ofono (instead Maemo cellular daemon) too... Currently we are using closed Nokia proprietary CAL library. Steps 1) and 2) needs closed library, step 4) needs dbus call. In current state I do not see way to integrate it into kernel. And because wl1251 currently uses request_firmware() to load those nvs data I think it is still the best way how to handle it... And IIRC there was already discussion about Nokia CAL parser in kernel and it was declined. > The firmware_class already supports a number of path prefixes it > traverses looking for the requested firmware. So I was thinking about > adding a hashtable in which a platform driver can add firmware which > are stored in the hashtable using the hashed firmware name. Upon a > firmware request from the driver we could check the hashtable before > traversing the path prefixes on VFS. The obvious problem is that the > request may come before the firmware is added to the hashtable. Just > wanted to pitch the idea first and hear what others think about it > and maybe someone has a nice solution for this problem. Fingers > crossed :-p > > > In case userspace helper fails request_firmware_prefer_user() still > > try to load data file directly from VFS as fallback mechanism. > > > > On Nokia N900 device which has wl1251 chip, NVS calibration data > > are stored in CAL nand partition. CAL is proprietary Nokia > > key/value format for nand devices. > > With the firmware hashtable api on N900 a platform driver could > interpret the CAL data in the nand partition and provide it through > the firmware_class. > > > With this patch it is finally possible to load correct model > > specific NVS calibration data for Nokia N900. > > But on other devices that use wl1251, but for instance have no > userspace helper the request to userspace will fail (after 60 sec?) > and try VFS after that. Maybe not so nice. Currently support for those devices is broken (like for N900) as without proper NVS data they do not work correctly... > You should consider other device configurations. Not just N900. I do not have any other wl1251 devices. I know that pandora has wl1251 too, but it has wl1251 with eeprom where is stored NVS. And in this case request_firmware() is not used there. > Regards, > Arend > > > Signed-off-by: Pali Rohár <pali.ro...@gmail.com> > > --- > > > > drivers/net/wireless/ti/wl1251/Kconfig |1 + > > drivers/net/wireless/ti/wl1251/main.c |2 +- > > 2 files changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/ti/wl1251/Kconfig > > b/drivers/net/wireless/ti/wl1251/Kconfig index 7142ccf..affe154 > > 100644 > > --- a/drivers/net/wireless/ti/wl1251/Kconfig > > +++ b/drivers/net/wireless/ti/wl1251/Kconfig > > @@ -2,6 +2,7 @@ config WL1251 > > > > tristate "TI wl1251 driver support" > > depends on MAC80211 > > select FW_LOADER > &
Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data
On Saturday 24 December 2016 19:17:30 Pavel Machek wrote:
> Hi!
>
> > In case there is no valid MAC address kernel generates random one.
> > This patch propagate this generated MAC address back to NVS data
> > which will be uploaded to wl1251 chip. So HW would have same MAC
> > address as linux kernel uses.
> >
> > return 0;
> >
> > }
> >
> > +static int wl1251_write_nvs_mac(struct wl1251 *wl)
> > +{
>
> The name is quite confusing, this sounds like writing into
> non-volatile storage.
>
> > + int i;
> > +
> > + if (wl->nvs_len < 0x24)
> > + return -ENODATA;
> > +
> > + /* length is 2 and data address is 0x546c (mask is 0xfffe) */
>
> You don't actually check for the mask.
It is quite complicated. { 0x6d, 0x54 } (= 0x546d) in data represent
address 0x546c and content are data. You need to apply mask 0xfffe for
0x546d and you get address where data will be written (so 0x546c).
> > + if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b]
> > != 0x54) + return -EINVAL;
>
> You have two copies of these. Does it make sense to move it to helper
> function?
I'm thinking if checks is really needed. But probably moving it to
separate function is good idea.
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH 5/6] wl1251: Parse and use MAC address from supplied NVS data
On Saturday 24 December 2016 19:14:21 Pavel Machek wrote:
> On Sat 2016-12-24 17:53:00, Pali Rohár wrote:
> > @@ -1581,10 +1598,16 @@ int wl1251_init_ieee80211(struct wl1251
> > *wl)
> >
> > wl->hw->queues = 4;
> >
> > + if (wl->nvs == NULL && !wl->use_eeprom) {
> > + ret = wl1251_fetch_nvs(wl);
> > + if (ret < 0)
> > + goto out;
> > + }
>
> Is goto out here good idea? IMNSHO it is copy bug, it should
> just proceed with generating random address.
No, goto is correct here. wl1251 cannot be initialized without NVS data.
And when fetching (from userspace) fails it is fatal error.
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid
On Saturday 24 December 2016 19:08:54 Pavel Machek wrote: > On Sat 2016-12-24 17:52:59, Pali Rohár wrote: > > Before this patch driver generated random MAC address every time > > when was doing initialization. And after that random MAC address > > could be overwritten with fixed one if provided. > > Before this patch, driver generated random MAC address every time it > was initialized. After that random MAC address could be overwritten > with fixed one, if provided. > > > This patch changes order. First it tries to read fixed MAC address > > and if it fails then driver generates random MAC address. > > I don't quite get where the advantage is supposed to be. Is it that > "use_eeprom" is set, but reading fails? Random bytes are read from kernel only if random MAC address is needed. And in wl->mac_addr is always either invalid address or permanenent mac address which will be used. Without patch in wl->mac_addr can be random temporary address for some time... > The only case where this helps is if wl1251_read_eeprom_mac() > succeeds but reads invalid address. > > > Signed-off-by: Pali Rohár <pali.ro...@gmail.com> > > Acked-by: Pavel Machek <pa...@ucw.cz> -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
[PATCH 6/6] wl1251: Set generated MAC address back to NVS data
In case there is no valid MAC address kernel generates random one. This
patch propagate this generated MAC address back to NVS data which will be
uploaded to wl1251 chip. So HW would have same MAC address as linux kernel
uses.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/net/wireless/ti/wl1251/main.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/net/wireless/ti/wl1251/main.c
b/drivers/net/wireless/ti/wl1251/main.c
index 1454ba2..895ae05 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -1555,6 +1555,24 @@ static int wl1251_read_nvs_mac(struct wl1251 *wl)
return 0;
}
+static int wl1251_write_nvs_mac(struct wl1251 *wl)
+{
+ int i;
+
+ if (wl->nvs_len < 0x24)
+ return -ENODATA;
+
+ /* length is 2 and data address is 0x546c (mask is 0xfffe) */
+ if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b] !=
0x54)
+ return -EINVAL;
+
+ /* MAC is stored in reverse order */
+ for (i = 0; i < ETH_ALEN; i++)
+ wl->nvs[0x1c + i] = wl->mac_addr[ETH_ALEN - i - 1];
+
+ return 0;
+}
+
static int wl1251_register_hw(struct wl1251 *wl)
{
int ret;
@@ -1620,6 +1638,8 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
memcpy(wl->mac_addr, nokia_oui, 3);
get_random_bytes(wl->mac_addr + 3, 3);
+ if (!wl->use_eeprom)
+ wl1251_write_nvs_mac(wl);
wl1251_warning("MAC address in eeprom or nvs data is not
valid");
wl1251_warning("Setting random MAC address: %pM", wl->mac_addr);
}
--
1.7.9.5
[PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid
Before this patch driver generated random MAC address every time when was
doing initialization. And after that random MAC address could be
overwritten with fixed one if provided.
This patch changes order. First it tries to read fixed MAC address and if
it fails then driver generates random MAC address.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/net/wireless/ti/wl1251/main.c | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/main.c
b/drivers/net/wireless/ti/wl1251/main.c
index 8971b64..c3fa0b6 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -1582,7 +1582,24 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
wl->hw->queues = 4;
if (wl->use_eeprom)
- wl1251_read_eeprom_mac(wl);
+ ret = wl1251_read_eeprom_mac(wl);
+ else
+ ret = -EINVAL;
+
+ if (ret == 0 && !is_valid_ether_addr(wl->mac_addr))
+ ret = -EINVAL;
+
+ if (ret < 0) {
+ /*
+* In case our MAC address is not correctly set,
+* we use a random but Nokia MAC.
+*/
+ static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
+ memcpy(wl->mac_addr, nokia_oui, 3);
+ get_random_bytes(wl->mac_addr + 3, 3);
+ wl1251_warning("MAC address in eeprom or nvs data is not
valid");
+ wl1251_warning("Setting random MAC address: %pM", wl->mac_addr);
+ }
ret = wl1251_register_hw(wl);
if (ret)
@@ -1623,7 +1640,6 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
struct ieee80211_hw *hw;
struct wl1251 *wl;
int i;
- static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
hw = ieee80211_alloc_hw(sizeof(*wl), _ops);
if (!hw) {
@@ -1674,13 +1690,6 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
INIT_WORK(>irq_work, wl1251_irq_work);
INIT_WORK(>tx_work, wl1251_tx_work);
- /*
-* In case our MAC address is not correctly set,
-* we use a random but Nokia MAC.
-*/
- memcpy(wl->mac_addr, nokia_oui, 3);
- get_random_bytes(wl->mac_addr + 3, 3);
-
wl->state = WL1251_STATE_OFF;
mutex_init(>mutex);
--
1.7.9.5
[PATCH 5/6] wl1251: Parse and use MAC address from supplied NVS data
This patch implements parsing MAC address from NVS data which are sent to
wl1251 chip. Calibration NVS data could contain valid MAC address and it
will be used instead randomly generated.
This patch also move code for requesting NVS data from userspace to driver
initialization code to make sure that NVS data will be there at time when
permanent MAC address is needed.
Calibration NVS data for wl1251 are model specific. Every one device with
wl1251 chip should have been calibrated in factory and needs to provide own
calibration data.
Default example wl1251-nvs.bin data found in linux-firmware repository and
contains MAC address 00:00:20:07:03:09. So this MAC address is marked as
invalid as it is not real device specific address, just example one.
Format of calibration NVS data can be found at:
http://notaz.gp2x.de/misc/pnd/wl1251/nvs_map.txt
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/net/wireless/ti/wl1251/main.c | 39 ++---
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/main.c
b/drivers/net/wireless/ti/wl1251/main.c
index c3fa0b6..1454ba2 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -205,13 +205,6 @@ static int wl1251_chip_wakeup(struct wl1251 *wl)
goto out;
}
- if (wl->nvs == NULL && !wl->use_eeprom) {
- /* No NVS from netlink, try to get it from the filesystem */
- ret = wl1251_fetch_nvs(wl);
- if (ret < 0)
- goto out;
- }
-
out:
return ret;
}
@@ -1538,6 +1531,30 @@ static int wl1251_read_eeprom_mac(struct wl1251 *wl)
return 0;
}
+static int wl1251_read_nvs_mac(struct wl1251 *wl)
+{
+ u8 mac[ETH_ALEN];
+ int i;
+
+ if (wl->nvs_len < 0x24)
+ return -ENODATA;
+
+ /* length is 2 and data address is 0x546c (mask is 0xfffe) */
+ if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b] !=
0x54)
+ return -EINVAL;
+
+ /* MAC is stored in reverse order */
+ for (i = 0; i < ETH_ALEN; i++)
+ mac[i] = wl->nvs[0x1c + ETH_ALEN - i - 1];
+
+ /* 00:00:20:07:03:09 is in default example wl1251-nvs.bin, so invalid */
+ if (ether_addr_equal_unaligned(mac, "\x00\x00\x20\x07\x03\x09"))
+ return -EINVAL;
+
+ memcpy(wl->mac_addr, mac, ETH_ALEN);
+ return 0;
+}
+
static int wl1251_register_hw(struct wl1251 *wl)
{
int ret;
@@ -1581,10 +1598,16 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
wl->hw->queues = 4;
+ if (wl->nvs == NULL && !wl->use_eeprom) {
+ ret = wl1251_fetch_nvs(wl);
+ if (ret < 0)
+ goto out;
+ }
+
if (wl->use_eeprom)
ret = wl1251_read_eeprom_mac(wl);
else
- ret = -EINVAL;
+ ret = wl1251_read_nvs_mac(wl);
if (ret == 0 && !is_valid_ether_addr(wl->mac_addr))
ret = -EINVAL;
--
1.7.9.5
[PATCH 1/6] firmware: Add request_firmware_prefer_user() function
This function works pretty much like request_firmware(), but it prefer
usermode helper. If usermode helper fails then it fallback to direct
access. Useful for dynamic or model specific firmware data.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/base/firmware_class.c | 45 +++--
include/linux/firmware.h |9 +
2 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 22d1760..6a8c261 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -119,6 +119,11 @@ static inline long firmware_loading_timeout(void)
#endif
#define FW_OPT_NO_WARN (1U << 3)
#define FW_OPT_NOCACHE (1U << 4)
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+#define FW_OPT_PREFER_USER (1U << 5)
+#else
+#define FW_OPT_PREFER_USER 0
+#endif
struct firmware_cache {
/* firmware_buf instance will be added into the below list */
@@ -1169,13 +1174,26 @@ static int assign_firmware_buf(struct firmware *fw,
struct device *device,
}
}
- ret = fw_get_filesystem_firmware(device, fw->priv);
+ if (opt_flags & FW_OPT_PREFER_USER) {
+ ret = fw_load_from_user_helper(fw, name, device, opt_flags,
timeout);
+ if (ret && !(opt_flags & FW_OPT_NO_WARN)) {
+ dev_warn(device,
+"User helper firmware load for %s failed with
error %d\n",
+name, ret);
+ dev_warn(device, "Falling back to direct firmware
load\n");
+ }
+ } else {
+ ret = -EINVAL;
+ }
+
+ if (ret)
+ ret = fw_get_filesystem_firmware(device, fw->priv);
if (ret) {
if (!(opt_flags & FW_OPT_NO_WARN))
dev_warn(device,
"Direct firmware load for %s failed with error
%d\n",
name, ret);
- if (opt_flags & FW_OPT_USERHELPER) {
+ if ((opt_flags & FW_OPT_USERHELPER) && !(opt_flags &
FW_OPT_PREFER_USER)) {
dev_warn(device, "Falling back to user helper\n");
ret = fw_load_from_user_helper(fw, name, device,
opt_flags, timeout);
@@ -1287,6 +1305,29 @@ int request_firmware_direct(const struct firmware
**firmware_p,
EXPORT_SYMBOL(request_firmware_into_buf);
/**
+ * request_firmware_prefer_user: - prefer usermode helper for loading firmware
+ * @firmware_p: pointer to firmware image
+ * @name: name of firmware file
+ * @device: device for which firmware is being loaded
+ *
+ * This function works pretty much like request_firmware(), but it prefer
+ * usermode helper. If usermode helper fails then it fallback to direct access.
+ * Useful for dynamic or model specific firmware data.
+ **/
+int request_firmware_prefer_user(const struct firmware **firmware_p,
+ const char *name, struct device *device)
+{
+ int ret;
+
+ __module_get(THIS_MODULE);
+ ret = _request_firmware(firmware_p, name, device, NULL, 0,
+ FW_OPT_UEVENT | FW_OPT_PREFER_USER);
+ module_put(THIS_MODULE);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(request_firmware_prefer_user);
+
+/**
* release_firmware: - release the resource associated with a firmware image
* @fw: firmware resource to release
**/
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index b1f9f0c..01f7a85 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -47,6 +47,8 @@ int request_firmware_nowait(
void (*cont)(const struct firmware *fw, void *context));
int request_firmware_direct(const struct firmware **fw, const char *name,
struct device *device);
+int request_firmware_prefer_user(const struct firmware **fw, const char *name,
+struct device *device);
int request_firmware_into_buf(const struct firmware **firmware_p,
const char *name, struct device *device, void *buf, size_t size);
@@ -77,6 +79,13 @@ static inline int request_firmware_direct(const struct
firmware **fw,
return -EINVAL;
}
+static inline int request_firmware_prefer_user(const struct firmware **fw,
+ const char *name,
+ struct device *device)
+{
+ return -EINVAL;
+}
+
static inline int request_firmware_into_buf(const struct firmware **firmware_p,
const char *name, struct device *device, void *buf, size_t size)
{
--
1.7.9.5
[PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
NVS calibration data for wl1251 are model specific. Every one device with
wl1251 chip has different and calibrated in factory.
Not all wl1251 chips have own EEPROM where are calibration data stored. And
in that case there is no "standard" place. Every device has stored them on
different place (some in rootfs file, some in dedicated nand partition,
some in another proprietary structure).
Kernel wl1251 driver cannot support every one different storage decided by
device manufacture so it will use request_firmware_prefer_user() call for
loading NVS calibration data and userspace helper will be responsible to
prepare correct data.
In case userspace helper fails request_firmware_prefer_user() still try to
load data file directly from VFS as fallback mechanism.
On Nokia N900 device which has wl1251 chip, NVS calibration data are stored
in CAL nand partition. CAL is proprietary Nokia key/value format for nand
devices.
With this patch it is finally possible to load correct model specific NVS
calibration data for Nokia N900.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/net/wireless/ti/wl1251/Kconfig |1 +
drivers/net/wireless/ti/wl1251/main.c |2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ti/wl1251/Kconfig
b/drivers/net/wireless/ti/wl1251/Kconfig
index 7142ccf..affe154 100644
--- a/drivers/net/wireless/ti/wl1251/Kconfig
+++ b/drivers/net/wireless/ti/wl1251/Kconfig
@@ -2,6 +2,7 @@ config WL1251
tristate "TI wl1251 driver support"
depends on MAC80211
select FW_LOADER
+ select FW_LOADER_USER_HELPER
select CRC7
---help---
This will enable TI wl1251 driver support. The drivers make
diff --git a/drivers/net/wireless/ti/wl1251/main.c
b/drivers/net/wireless/ti/wl1251/main.c
index 208f062..24f8866 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -110,7 +110,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
struct device *dev = wiphy_dev(wl->hw->wiphy);
int ret;
- ret = request_firmware(, WL1251_NVS_NAME, dev);
+ ret = request_firmware_prefer_user(, WL1251_NVS_NAME, dev);
if (ret < 0) {
wl1251_error("could not get nvs file: %d", ret);
--
1.7.9.5
[PATCH 3/6] wl1251: Update wl->nvs_len after wl->nvs is valid
In case kmemdup fails thne wl->nvs_len will contains invalid non-zero size.
This patch fixes it.
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
drivers/net/wireless/ti/wl1251/main.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/main.c
b/drivers/net/wireless/ti/wl1251/main.c
index 24f8866..8971b64 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -124,8 +124,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
goto out;
}
- wl->nvs_len = fw->size;
- wl->nvs = kmemdup(fw->data, wl->nvs_len, GFP_KERNEL);
+ wl->nvs = kmemdup(fw->data, fw->size, GFP_KERNEL);
if (!wl->nvs) {
wl1251_error("could not allocate memory for the nvs file");
@@ -133,6 +132,8 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
goto out;
}
+ wl->nvs_len = fw->size;
+
ret = 0;
out:
--
1.7.9.5
[PATCH 0/6] wl1251: Fix MAC address for Nokia N900
This patch series fix processing MAC address for wl1251 chip found in Nokia N900. Pali Rohár (6): firmware: Add request_firmware_prefer_user() function wl1251: Use request_firmware_prefer_user() for loading NVS calibration data wl1251: Update wl->nvs_len after wl->nvs is valid wl1251: Generate random MAC address only if driver does not have valid wl1251: Parse and use MAC address from supplied NVS data wl1251: Set generated MAC address back to NVS data drivers/base/firmware_class.c | 45 +++- drivers/net/wireless/ti/wl1251/Kconfig |1 + drivers/net/wireless/ti/wl1251/main.c | 91 +--- include/linux/firmware.h |9 4 files changed, 125 insertions(+), 21 deletions(-) -- 1.7.9.5
Re: wl1251 & mac address & calibration data
On Tuesday 20 December 2016 17:56:58 Tony Lindgren wrote: > * Kalle Valo <kv...@codeaurora.org> [161220 03:47]: > > Arend Van Spriel <arend.vanspr...@broadcom.com> writes: > > > On 18-12-2016 13:09, Pali Rohár wrote: > > >> File wl1251-nvs.bin is provided by linux-firmware package and > > >> contains default data which should be overriden by model > > >> specific calibrated data. > > > > > > Ah. Someone thought it was a good idea to provide the "one ring > > > to rule them all". Nice. > > > > Yes, that was a bad idea. wl1251-nvs.bin in linux-firmware.git > > should be renamed to wl1251-nvs.bin.example, or something like > > that, as it should be only installed to a real system only if > > there's no real calibration data available (only for developers to > > use, not real users). > > Makes sense to me. Note that with the recent changes to wlcore, we > can now easily provide board specific calibration firmware simply by > adding a new compatible value. So for n900, we could have something > like compatible = "ti,wl1251-n900" and have it point to n900 > specific calibration file wl1251-nvs-n900.bin. Of course this won't > help with the mac address, or any of the device specific data.. > > That is assuming the calibration values are the same for each similar > device and don't have to be generated for each device. And naturally > wl1251 needs simlar changes done to make use of devices specific > calibration files. > > Regards, > > Tony As wrote in another thread "wl1251 NVS calibration data format" calibration data for wl1251 (wl1251-nvs.bin) contains also MAC address, which kernel sends to wl1251 chip. Kernel just do not use it. So... my idea now is: 1) extend request_firmware function family with ability to use userspace helper first and fallback to VFS 2) teach wl1251.ko to parse MAC address from wl1251-nvs.bin and use it (in case it is not empty or 00:00:20:07:03:09 which is in that example linux-firmware package) 3) write Nokia n900 specific userspace helper for providing data when kernel requests wl1251-nvs.bin. So userspace helper reads MAC address and calibration data from CAL, place MAC address into calibration data and send put it into kernel. Are you OK with this idea? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: wl1251 & mac address & calibration data
On Sunday 18 December 2016 12:54:00 Arend Van Spriel wrote:
> On 18-12-2016 12:04, Pali Rohár wrote:
> > On Sunday 18 December 2016 11:49:53 Arend Van Spriel wrote:
> >> On 16-12-2016 11:40, Pali Rohár wrote:
> >>> On Friday 16 December 2016 08:25:44 Daniel Wagner wrote:
> >>>> On 12/16/2016 03:03 AM, Luis R. Rodriguez wrote:
> >>>>> For the new API a solution for "fallback mechanisms" should be
> >>>>> clean though and I am looking to stay as far as possible from
> >>>>> the existing mess. A solution to help both the old API and new
> >>>>> API is possible for the "fallback mechanism" though -- but for
> >>>>> that I can only refer you at this point to some of Daniel
> >>>>> Wagner and Tom Gunderson's firmwared deamon prospect. It
> >>>>> should help pave the way for a clean solution and help address
> >>>>> other stupid issues.
> >>>>
> >>>> The firmwared project is hosted here
> >>>>
> >>>> https://github.com/teg/firmwared
> >>>>
> >>>> As Luis pointed out, firmwared relies on
> >>>> FW_LOADER_USER_HELPER_FALLBACK, which is not enabled by default.
> >>>
> >>> I know. But it does not mean that I cannot enable this option at
> >>> kernel compile time.
> >>>
> >>> Bigger problem is that currently request_firmware() first try to
> >>> load firmware directly from VFS and after that (if fails)
> >>> fallback to user helper.
> >>>
> >>> So I would need to extend kernel firmware code with new function
> >>> (or flag) to not use VFS and try only user mode helper.
> >>
> >> Why do you need the user-mode helper anyway. This is all static
> >> data, right?
> >
> > Those are static data, but device specific!
>
> So what?
>
> >> So why not cook up a firmware file in user-space once and put
> >> it in /lib/firmware for the driver to request directly.
> >
> > 1. Violates FHS
>
> How?
>
> > 2. Does not work for readonly /, readonly /lib, readonly
> > /lib/firmware
>
> Que?
>
> > 3. Backup & restore of rootfs between same devices does not work
> > (as rootfs now contains device specific data).
>
> True.
>
> > 4. Sharing one rootfs (either via nfs or other technology) does not
> > work for more devices (even in state when rootfs is used only by
> > one device at one time).
>
> Indeed.
>
> > And it is common that N900 developers have rootfs in laptop and via
> > usb (cdc_ether) exports it over nfs to N900 device and boot
> > system. It basically break booting from one nfs-exported rootfs,
> > as that export become model specific...
>
> These are all you choices and more a logistic issue. If your take is
> that udev is the way to solve those, fine by me.
>
> >> Seems a bit
> >> overkill to have a {e,}udev or whatever daemon running if the
> >> result is always the same. Just my 2 cents.
> >
> > No it is not. It will break couple of other things in Linux and
> > device
>
> Now I am curious. What "couple of other things" will be broken.
>
> > and model specific calibration data should not be in /lib/firmware!
> > That directory is used for firmware files, not calibration.
>
> What is "firmware"? Really. These are binary blobs required to make
> the device work. And guess what, your device needs calibration data.
> Why make the distinction.
>
> Regards,
> Arend
File wl1251-nvs.bin is provided by linux-firmware package and contains
default data which should be overriden by model specific calibrated
data.
But overwriting that one file is not possible as it next update of
linux-firmware package will overwrite it back. It break any normal usage
of package management.
Also it is ridiculously broken by design if some "boot" files needs to
be overwritten to initialize hardware properly. To not break booting you
need to overwrite that file before first boot. But without booting
device you cannot read calibration data. So some hack with autoreboot
after boot is needed. And how to detect that we have real overwritten
calibration data and not default one from linux-firmware? Any heuristic
or checks will be broken here. And no, nothing like you need to reboot
your device now (and similar concept) from windows world is not
accepted.
"firmware" is one for chip. Any N900 device with wl1251 chip needs
exactly same firmware "wl1251-fw.bin". But every N900 needs different
calibration data which is not firmware.
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
Re: wl1251 & mac address & calibration data
On Sunday 18 December 2016 11:49:53 Arend Van Spriel wrote:
> On 16-12-2016 11:40, Pali Rohár wrote:
> > On Friday 16 December 2016 08:25:44 Daniel Wagner wrote:
> >> On 12/16/2016 03:03 AM, Luis R. Rodriguez wrote:
> >>> For the new API a solution for "fallback mechanisms" should be
> >>> clean though and I am looking to stay as far as possible from the
> >>> existing mess. A solution to help both the old API and new API is
> >>> possible for the "fallback mechanism" though -- but for that I
> >>> can only refer you at this point to some of Daniel Wagner and
> >>> Tom Gunderson's firmwared deamon prospect. It should help pave
> >>> the way for a clean solution and help address other stupid
> >>> issues.
> >>
> >> The firmwared project is hosted here
> >>
> >> https://github.com/teg/firmwared
> >>
> >> As Luis pointed out, firmwared relies on
> >> FW_LOADER_USER_HELPER_FALLBACK, which is not enabled by default.
> >
> > I know. But it does not mean that I cannot enable this option at
> > kernel compile time.
> >
> > Bigger problem is that currently request_firmware() first try to
> > load firmware directly from VFS and after that (if fails) fallback
> > to user helper.
> >
> > So I would need to extend kernel firmware code with new function
> > (or flag) to not use VFS and try only user mode helper.
>
> Why do you need the user-mode helper anyway. This is all static data,
> right?
Those are static data, but device specific!
> So why not cook up a firmware file in user-space once and put
> it in /lib/firmware for the driver to request directly.
1. Violates FHS
2. Does not work for readonly /, readonly /lib, readonly /lib/firmware
3. Backup & restore of rootfs between same devices does not work (as
rootfs now contains device specific data).
4. Sharing one rootfs (either via nfs or other technology) does not work
for more devices (even in state when rootfs is used only by one device
at one time).
And it is common that N900 developers have rootfs in laptop and via usb
(cdc_ether) exports it over nfs to N900 device and boot system. It
basically break booting from one nfs-exported rootfs, as that export
become model specific...
> Seems a bit
> overkill to have a {e,}udev or whatever daemon running if the result
> is always the same. Just my 2 cents.
No it is not. It will break couple of other things in Linux and device
and model specific calibration data should not be in /lib/firmware! That
directory is used for firmware files, not calibration.
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
Re: wl1251 & mac address & calibration data
On Friday 16 December 2016 08:25:44 Daniel Wagner wrote: > On 12/16/2016 03:03 AM, Luis R. Rodriguez wrote: > > For the new API a solution for "fallback mechanisms" should be > > clean though and I am looking to stay as far as possible from the > > existing mess. A solution to help both the old API and new API is > > possible for the "fallback mechanism" though -- but for that I can > > only refer you at this point to some of Daniel Wagner and Tom > > Gunderson's firmwared deamon prospect. It should help pave the way > > for a clean solution and help address other stupid issues. > > The firmwared project is hosted here > > https://github.com/teg/firmwared > > As Luis pointed out, firmwared relies on > FW_LOADER_USER_HELPER_FALLBACK, which is not enabled by default. I know. But it does not mean that I cannot enable this option at kernel compile time. Bigger problem is that currently request_firmware() first try to load firmware directly from VFS and after that (if fails) fallback to user helper. So I would need to extend kernel firmware code with new function (or flag) to not use VFS and try only user mode helper. > I > don't see any reason why firmwared should not also support loading > calibration data. If we find a sound way to do this. It can, but why should I use another daemon for firmware loading as non-systemd version of udev (and eudev fork) support firmware loading? I think I stay with udev/eudev. > As you can see from the commit history it is a pretty young project > and more ore less reanimation of the old udev firmware loader > feature. We are getting int into shape, adding integration tests > etc. > > The main motivation for this project is the get movement back in > stuck discussion on the firmware loader API. Luis was very busy > writing up all the details on the current situation and purely from > the amount of documentation need to describe the API you can tell > something is awry. > > Thanks, > Daniel -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: wl1251 & mac address & calibration data
On Friday 16 December 2016 03:03:19 Luis R. Rodriguez wrote: > On Thu, Dec 15, 2016 at 2:12 PM, Arend Van Spriel > > <arend.vanspr...@broadcom.com> wrote: > > On 15-12-2016 16:33, Pali Rohár wrote: > >> On Thu Dec 15 09:18:44 2016 Kalle Valo <kv...@codeaurora.org> > >> wrote: > >>> (Adding Luis because he has been working on request_firmware() > >>> lately) > >>> > >>> Pali Rohár <pali.ro...@gmail.com> writes: > >>>>>> So no, there is no argument against... request_firmware() in > >>>>>> fallback mode with userspace helper is by design blocking and > >>>>>> waiting for userspace. But waiting for some change in DTS in > >>>>>> kernel is just nonsense. > >>>>> > >>>>> I would just mark the wlan device with status = "disabled" and > >>>>> enable it in the overlay together with adding the NVS & MAC > >>>>> info. > >>>> > >>>> So if you think that this solution make sense, we can wait what > >>>> net wireless maintainers say about it... > >>>> > >>>> For me it looks like that solution can be: > >>>> > >>>> extending request_firmware() to use only userspace helper > >>> > >>> I haven't followed the discussion very closely but this is my > >>> preference what drivers should do: > >>> > >>> 1) First the driver should do try to get the calibration data and > >>> mac > >>> > >>>address from the device tree. > >> > >> Ok, but there is no (dynamic, device specific) data in DTS for > >> N900. So 1) is noop. > > > > Uhm. What do you mean? You can propose a patch to the DT bindings > > [1] to get it in there and create your N900 DTB or am I missing > > something here. Are there hardware restrictions that do not allow > > you to boot with your own DTB. > > > >>> 2) If they are not in DT the driver should retrieve the > >>> calibration data > >>> > >>>with request_firmware(). BUT with an option for user space > >>>to implement that with a helper script so that the data > >>>can be created dynamically, which I believe openwrt does > >>>with ath10k calibration data right now. > >> > >> Currently there is flag for request_firmware() that it should > >> fallback to user helper if direct VFS access not find needed > >> firmware. > >> > >> But this flag is not suitable as /lib/firmware already provides > >> default (not device specific) calibration data. > >> > >> So I would suggest to add another flag/function which will primary > >> use user helper. > > > > I recall Luis saying that user-mode helper (fallback) should be > > discouraged, because there is no assurance that there is a > > user-mode helper so you might just be pissing in the wind. > > There's tons of issues with the current status quo of the so called > "firmware usermode helper". To start off with its a complete > misnomer, the kernel's usermode helper infrastructure is implemented > in lib/kmod.c and it provides an API to help call out to userspace > some helper for you. That's the real kernel usermode helper. In so > far as the firmware_class.c driver is concerned -- it only makes use > of the kernel user mode helper as an option if you have > CONFIG_UEVENT_HELPER_PATH set to help address kobject uevents. Most > distributions do not use this, and back in the day systemd udev, and > prior to that hotplug used to process firmware kobject uevents. > systemd udev is long gone. Gone. This kobject uevents really are a > "fallback mechanism" for firmware only -- if cached firmware, > built-in firmware, or direct filesystem lookup fails. These each are > their own beast. Finally kobject uevents are only one of the > fallback > mechanisms, "custom fallback mechanisms" are the other option and its > what you and others seem to describe to want for these sorts of > custom things. > > There are issues with the existing request_firmware() API in that > everyone and their mother keeps extending it with stupid small API > extensions to do yet-another-tweak, and then having to go and change > tons of drivers. Or a new API call added for just one custom knob. > Naturally this is all plain dumb, so yet-another-API call or new > argument is not going to help us. We don't have "flags"
Re: wl1251 & mac address & calibration data
On Thursday 15 December 2016 21:12:47 Arend Van Spriel wrote: > On 15-12-2016 16:33, Pali Rohár wrote: > > On Thu Dec 15 09:18:44 2016 Kalle Valo <kv...@codeaurora.org> wrote: > >> (Adding Luis because he has been working on request_firmware() > >> lately) > >> > >> Pali Rohár <pali.ro...@gmail.com> writes: > >>>>> So no, there is no argument against... request_firmware() in > >>>>> fallback mode with userspace helper is by design blocking and > >>>>> waiting for userspace. But waiting for some change in DTS in > >>>>> kernel is just nonsense. > >>>> > >>>> I would just mark the wlan device with status = "disabled" and > >>>> enable it in the overlay together with adding the NVS & MAC > >>>> info. > >>> > >>> So if you think that this solution make sense, we can wait what > >>> net wireless maintainers say about it... > >>> > >>> For me it looks like that solution can be: > >>> > >>> extending request_firmware() to use only userspace helper > >> > >> I haven't followed the discussion very closely but this is my > >> preference what drivers should do: > >> > >> 1) First the driver should do try to get the calibration data and > >> mac > >> > >>address from the device tree. > > > > Ok, but there is no (dynamic, device specific) data in DTS for > > N900. So 1) is noop. > > Uhm. What do you mean? You can propose a patch to the DT bindings [1] > to get it in there and create your N900 DTB or am I missing > something here. Are there hardware restrictions that do not allow > you to boot with your own DTB. What is [1]? N900's bootloader does not support DTB and it does not pass any DTB. It boots kernel in ATAGs mode. What we are doing is appending DTB compiled from kernel sources to end of zImage. But that appended DTB cannot contains device specific nodes (e.g. calibration data for device) as zImage is there for any N900 device, not just only one. > >> 2) If they are not in DT the driver should retrieve the > >> calibration data > >> > >>with request_firmware(). BUT with an option for user space > >>to implement that with a helper script so that the data > >>can be created dynamically, which I believe openwrt does > >>with ath10k calibration data right now. > > > > Currently there is flag for request_firmware() that it should > > fallback to user helper if direct VFS access not find needed > > firmware. > > > > But this flag is not suitable as /lib/firmware already provides > > default (not device specific) calibration data. > > > > So I would suggest to add another flag/function which will primary > > use user helper. > > I recall Luis saying that user-mode helper (fallback) should be > discouraged, because there is no assurance that there is a user-mode > helper so you might just be pissing in the wind. The idea was to have > a dedicated API call that explicitly does the request towards > user-space. > > By the way, are we talking here about wl1251 device or driver as you > also mentioned wl12xx? I did not read the entire thread. Yes, we are talking about wl1251 device, which is in Nokia N900 and wl1251.ko and wl1251_spi.ko drivers. I mentioned wl12xx as it already uses similar approach with mac address via request_firmware(). And as those drivers are very similar plus from one manufactor and in same kernel folder I mentioned similar API for consistency... -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: wl1251 & mac address & calibration data
On Thu Dec 15 09:18:44 2016 Kalle Valo <kv...@codeaurora.org> wrote: > (Adding Luis because he has been working on request_firmware() lately) > > Pali Rohár <pali.ro...@gmail.com> writes: > > > > > So no, there is no argument against... request_firmware() in > > > > fallback mode with userspace helper is by design blocking and > > > > waiting for userspace. But waiting for some change in DTS in > > > > kernel is just nonsense. > > > > > > I would just mark the wlan device with status = "disabled" and > > > enable it in the overlay together with adding the NVS & MAC info. > > > > So if you think that this solution make sense, we can wait what net > > wireless maintainers say about it... > > > > For me it looks like that solution can be: > > > > extending request_firmware() to use only userspace helper > > I haven't followed the discussion very closely but this is my preference > what drivers should do: > > 1) First the driver should do try to get the calibration data and mac > address from the device tree. > Ok, but there is no (dynamic, device specific) data in DTS for N900. So 1) is noop. > 2) If they are not in DT the driver should retrieve the calibration data > with request_firmware(). BUT with an option for user space to > implement that with a helper script so that the data can be created > dynamically, which I believe openwrt does with ath10k calibration > data right now. Currently there is flag for request_firmware() that it should fallback to user helper if direct VFS access not find needed firmware. But this flag is not suitable as /lib/firmware already provides default (not device specific) calibration data. So I would suggest to add another flag/function which will primary use user helper. > > and load mac address also via request_firmware() either by appending > > it into NVS data or via separate call > > I'm not really fan of the idea providing permanent mac address through > request_firmware(). For example, how to handle multiple devices on the > same host, would there be a need for some kind of bus ids encoded to the > filename? And what about devices with multiple mac addresses? For N900 there is only one wl1251 device. And... wl12xx is already using appended MAC address in calibration data read by request firmware. So reason why I prefer similar usage also for wl1251. > I wish there would be a better way than request_firmware() to provide > the permanent mac addresses from user space (if device tree is not > available), I just don't know what that could be :) But if we would > start to use request_firmware() for this at least there should be a > wider concensus about that and it should be properly documented, just > like the device tree bindings. > > -- > Kalle Valo I do not know about any other, so reason why I'm asking :-) and there are my proposed solutions. If you (or any other) came up with better we can discuss about it :-) -- Pali Rohár pali.ro...@gmail.com
Re: wl1251 & mac address & calibration data
On Thursday 24 November 2016 19:46:01 Aaro Koskinen wrote: > Hi, > > On Thu, Nov 24, 2016 at 04:20:45PM +0100, Pali Rohár wrote: > > Proprietary, signed and closed bootloader NOLO does not support DT. > > So for booting you need to append DTS file to kernel image. > > > > U-Boot is optional and can be used as intermediate bootloader > > between NOLO and kernel. But still it has problems with reading > > from nand, so cannot read NVS data nor MAC address. > > You could use kexec to pass the fixed DT. > > A. IIRC it was broken for N900/omap3, no idea if somebody fixed it. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: wl1251 & mac address & calibration data
On Thursday 24 November 2016 19:11:39 Sebastian Reichel wrote: > Hi, > > On Thu, Nov 24, 2016 at 05:49:33PM +0100, Pali Rohár wrote: > > On Thursday 24 November 2016 17:08:30 Sebastian Reichel wrote: > > > On Thu, Nov 24, 2016 at 04:20:45PM +0100, Pali Rohár wrote: > > > > On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote: > > > > > On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote: > > > > > > On Thursday 24 November 2016 08:51:04 Pavel Machek wrote: > > > > > > > > > "ifconfig hw ether XX" normally sets the address. I > > > > > > > > > guess that's ioctl? > > > > > > > > > > > > > > > > This sets temporary address and it is ioctl. IIRC same > > > > > > > > as what ethtool uses. (ifconfig is already > > > > > > > > deprecated). > > > > > > > > > > > > > > > > > And I guess we should use similar mechanism for > > > > > > > > > permanent address. > > > > > > > > > > > > > > > > I'm not sure here... Above ioctl ↑↑↑ is for changing > > > > > > > > temporary mac address. But here we do not want to > > > > > > > > change permanent mac address. We want to tell kernel > > > > > > > > driver current permanent mac address which is stored > > > > > > > > > > > > > > Well... I'd still use similar mechanism :-). > > > > > > > > > > > > Thats problematic, because in time when wlan0 interface is > > > > > > registered into system and visible in ifconfig output it > > > > > > already needs to have permanent mac address assigned. > > > > > > > > > > > > We should assign permanent mac address before wlan0 of > > > > > > wl1251 is registered into system. > > > > > > > > > > You can just add the MAC address to the NVS data, which is > > > > > also required for the device initialization. > > > > > > > > NVS data file has fixed size, there is IIRC no place for it. > > > > > > > > But one of my suggestion was to use another request_firmware > > > > for MAC address. So this is similar to what you are proposing, > > > > as NVS data are loaded by request_firmware too... > > > > > > Just append it to NVS data and modify the size check accordingly? > > > > First that would mean to have request_firmware() function which > > will not use direct VFS access, but instead use userspace helper. > > Permanent MAC is device specific init data, NVS is device specific > init data => load together. > > The userspace helper stuff is only needed to get the data from > the NAND calibration area on the fly. But it is needed... and currently request_firmware() prefer direct VFS access... > > NVS data file with some default values (not suitable for usage) is > > already present in linux-firmware and available in > > /lib/firmware/... > > You mentioned NVS data is fixed size, so this should be enough? > > switch(loaded_size) > case IMAGE_SIZE + MAC_SIZE: > /* extract mac */ > /* fallthrough */ > case IMAGE_SIZE: > /* load NVS */ > break; > default: > /* fail */ > } > > > Also I'm not sure if such thing is allowed by license: > > https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmwar > > e.git/tree/LICENCE.ti-connectivity > > concating data is not a modification, otherwise you can't > put the file in a filesystem. Ok, if net maintainers agree. > > > > > I wonder if those information could be put into DT. Iirc some > > > > > network devices get their MAC address from DT. Maybe we can > > > > > add all NVS info to DT? How much data is it? > > > > > > > > Proprietary, signed and closed bootloader NOLO does not support > > > > DT. So for booting you need to append DTS file to kernel > > > > image. > > > > > > Yeah, so NOLO without U-Boot will depend on userspace to fixup > > > DT. > > > > > > > U-Boot is optional and can be used as intermediate bootloader > > > > between NOLO and kernel. But still it has problems with reading > > > > from nand, so cannot read NVS data nor MAC address. > > > > > > It may in the futu
Re: wl1251 & mac address & calibration data
On Thursday 24 November 2016 17:08:30 Sebastian Reichel wrote: > Hi, > > On Thu, Nov 24, 2016 at 04:20:45PM +0100, Pali Rohár wrote: > > On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote: > > > On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote: > > > > On Thursday 24 November 2016 08:51:04 Pavel Machek wrote: > > > > > > > "ifconfig hw ether XX" normally sets the address. I guess > > > > > > > that's ioctl? > > > > > > > > > > > > This sets temporary address and it is ioctl. IIRC same as > > > > > > what ethtool uses. (ifconfig is already deprecated). > > > > > > > > > > > > > And I guess we should use similar mechanism for permanent > > > > > > > address. > > > > > > > > > > > > I'm not sure here... Above ioctl ↑↑↑ is for changing > > > > > > temporary mac address. But here we do not want to change > > > > > > permanent mac address. We want to tell kernel driver > > > > > > current permanent mac address which is stored > > > > > > > > > > Well... I'd still use similar mechanism :-). > > > > > > > > Thats problematic, because in time when wlan0 interface is > > > > registered into system and visible in ifconfig output it > > > > already needs to have permanent mac address assigned. > > > > > > > > We should assign permanent mac address before wlan0 of wl1251 > > > > is registered into system. > > > > > > You can just add the MAC address to the NVS data, which is also > > > required for the device initialization. > > > > NVS data file has fixed size, there is IIRC no place for it. > > > > But one of my suggestion was to use another request_firmware for > > MAC address. So this is similar to what you are proposing, as NVS > > data are loaded by request_firmware too... > > Just append it to NVS data and modify the size check accordingly? First that would mean to have request_firmware() function which will not use direct VFS access, but instead use userspace helper. NVS data file with some default values (not suitable for usage) is already present in linux-firmware and available in /lib/firmware/... Also I'm not sure if such thing is allowed by license: https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.ti-connectivity > > > I wonder if those information could be put into DT. Iirc some > > > network devices get their MAC address from DT. Maybe we can add > > > all NVS info to DT? How much data is it? > > > > Proprietary, signed and closed bootloader NOLO does not support DT. > > So for booting you need to append DTS file to kernel image. > > Yeah, so NOLO without U-Boot will depend on userspace to fixup DT. > > > U-Boot is optional and can be used as intermediate bootloader > > between NOLO and kernel. But still it has problems with reading > > from nand, so cannot read NVS data nor MAC address. > > It may in the future? I already tried that, but I failed. I was not able to access N900's nand from u-boot. No idea where was problem... And if somebody fix onenand in u-boot, then needs to reimplement Nokia's proprietary parser of that partition where is stored NVS and mac address && make this support in upstream u-boot. So... I doubt it will be in any future. + no men power > > > Userspace application can add all those information to the DT > > > using a DT overlay. Also the u-boot could parse and add it at > > > some point in the future. > > > > In case when wl1251 is statically linked into kernel image, it is > > loaded and initialized before even userspace applications starts. > > > > So no... adding NVS data or MAC address into DT or DT overlay is > > not a solution. > > Actually with data loaded from DT you *can* load data quite early in > the boot process, while your suggestions always require userspace. > So you argument against yourself? You cannot add DTS in uboot (no support). And if you modify DTS on running kernel from userspace, then it is too late when wl1251 is statically linked into kernel image. wl1251 will not wait until some userspace modify DTS and add data... But request_firmware() in fallback mode *can* wait for userspace so wl1251 can postpone its operation until mdev/udev/whatever starts listening for events and push needed firmware data to kernel. So no, there is no argument against... request_firmware() in fallback mode with userspace helper is by design blocking and waiting for userspace. But waiting for some change in DST in kernel is just nonsense. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: wl1251 & mac address & calibration data
On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote: > Hi, > > On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote: > > On Thursday 24 November 2016 08:51:04 Pavel Machek wrote: > > > Hi! > > > > > > > > "ifconfig hw ether XX" normally sets the address. I guess that's > > > > > ioctl? > > > > > > > > This sets temporary address and it is ioctl. IIRC same as what ethtool > > > > uses. (ifconfig is already deprecated). > > > > > > > > > And I guess we should use similar mechanism for permanent > > > > > address. > > > > > > > > I'm not sure here... Above ioctl ↑↑↑ is for changing temporary mac > > > > address. But here we do not want to change permanent mac address. We > > > > want to tell kernel driver current permanent mac address which is > > > > stored > > > > > > Well... I'd still use similar mechanism :-). > > > > Thats problematic, because in time when wlan0 interface is registered > > into system and visible in ifconfig output it already needs to have > > permanent mac address assigned. > > > > We should assign permanent mac address before wlan0 of wl1251 is > > registered into system. > > You can just add the MAC address to the NVS data, which is also > required for the device initialization. NVS data file has fixed size, there is IIRC no place for it. But one of my suggestion was to use another request_firmware for MAC address. So this is similar to what you are proposing, as NVS data are loaded by request_firmware too... > I wonder if those information could be put into DT. Iirc some > network devices get their MAC address from DT. Maybe we can add > all NVS info to DT? How much data is it? Proprietary, signed and closed bootloader NOLO does not support DT. So for booting you need to append DTS file to kernel image. U-Boot is optional and can be used as intermediate bootloader between NOLO and kernel. But still it has problems with reading from nand, so cannot read NVS data nor MAC address. > Userspace application can add all those information to the DT > using a DT overlay. Also the u-boot could parse and add it at > some point in the future. In case when wl1251 is statically linked into kernel image, it is loaded and initialized before even userspace applications starts. So no... adding NVS data or MAC address into DT or DT overlay is not a solution. -- Pali Rohár pali.ro...@gmail.com
Re: wl1251 & mac address & calibration data
On Thursday 24 November 2016 08:51:04 Pavel Machek wrote: > Hi! > > > > "ifconfig hw ether XX" normally sets the address. I guess that's > > > ioctl? > > > > This sets temporary address and it is ioctl. IIRC same as what ethtool > > uses. (ifconfig is already deprecated). > > > > > And I guess we should use similar mechanism for permanent > > > address. > > > > I'm not sure here... Above ioctl ↑↑↑ is for changing temporary mac > > address. But here we do not want to change permanent mac address. We > > want to tell kernel driver current permanent mac address which is > > stored > > Well... I'd still use similar mechanism :-). Thats problematic, because in time when wlan0 interface is registered into system and visible in ifconfig output it already needs to have permanent mac address assigned. We should assign permanent mac address before wlan0 of wl1251 is registered into system. -- Pali Rohár pali.ro...@gmail.com
Re: wl1251 & mac address & calibration data
On Wednesday 23 November 2016 23:23:35 Pavel Machek wrote: > Hi! > > > > > As wl1251.ko does not accept mac_address as module parameter, > > > > such modprobe hook does not help -- as there is absolutely no > > > > way from userspace to set or change (permanent) mac address. > > > > > > Quoting modprobe.d manual: > > > > install modulename command... > > > > > > > > This command instructs modprobe to run your > > > > command instead of inserting the module in the > > > > kernel as normal. The command can be any shell > > > > command: this allows you to do any kind of > > > > complex processing you might wish. [...] > > > > I know. But this do not allow me to send mac address to kernel -- > > as kernel does not support such command yet (reason for my first > > question). > > Plus, this does not really work for cases where wl1251 is not a > module. Yes, this is another problem. > > > You can hook up a script that cooks up wl1251-nvs.bin (caldata, > > > macaddr) and then insmod the actual wl1251.ko module. Or you can > > > just cook up the nvs on first device boot and store it in > > > /lib/firmware (possibly overwriting the "generic" wl1251 from > > > linux-firmware). > > > > This is what I would like to prevent -- overwriting (possible > > readonly) system files with some device specific. It is really bad > > idea! > > Agreed. > > "ifconfig hw ether XX" normally sets the address. I guess that's > ioctl? This sets temporary address and it is ioctl. IIRC same as what ethtool uses. (ifconfig is already deprecated). > And I guess we should use similar mechanism for permanent > address. I'm not sure here... Above ioctl ↑↑↑ is for changing temporary mac address. But here we do not want to change permanent mac address. We want to tell kernel driver current permanent mac address which is stored in permanent mac address storage (in N900 case in mtd). Just like userspace helper as kernel driver do not have code which can read permanent mac address. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: wl1251 & mac address & calibration data
On Tuesday 22 November 2016 17:14:28 Michal Kazior wrote: > On 22 November 2016 at 16:31, Pali Rohár <pali.ro...@gmail.com> wrote: > > On Tuesday 22 November 2016 16:22:57 Michal Kazior wrote: > >> On 21 November 2016 at 16:51, Pali Rohár <pali.ro...@gmail.com> > >> wrote: > >> > On Friday 11 November 2016 18:20:50 Pali Rohár wrote: > >> >> Hi! I will open discussion about mac address and calibration > >> >> data for wl1251 wireless chip again... > >> >> > >> >> Problem: Mac address & calibration data for wl1251 chip on > >> >> Nokia N900 are stored on second nand partition (mtd1) in > >> >> special proprietary format which is used only for Nokia N900 > >> >> (probably on N8x0 and N9 too). Wireless driver wl1251.ko > >> >> cannot work without mac address and calibration data. > >> > >> Same problem applies to some ath9k/ath10k supported routers. Some > >> even carry mac address as implicit offset from ethernet mac > >> address. As far as I understand OpenWRT cooks cal blobs on first > >> boot prior to loading modules. > > > > So... wl1251 on Nokia N900 is not alone and this problem is there > > for more drivers and devices. Which means we should come up with > > some generic solution. > > This isn't particularly a problem for ath9k/ath10k. > > Let me give you more background on ath10k. > > ath10k devices can come with caldata and macaddr stored in their > OTP/EEPROM. In that case a generic "template" board file is used. > Userspace doesn't need to do anything special. > > Some vendors however decide to use flash partition to store caldata. > In that case ath10k expects userspace to prepare > cal-$bus-$devname.bin files, each for a different radio (you can > have multiple radios on a system). > > Now translating this for wl1251 I would expect it should also use > something like wl1251-nvs-sdio-0x0001.bin for devices like N900 that > have caldata on flash partition (instead of the generic > wl1251-nvs.bin). I'm not sure if wl1251-nvs.bin is something > comparable to (the generic) board.bin ath10k has though. Maybe the > entire idea behind wl1251-nvs.bin is flawed as it's supposed to be > device specific and is oblivious to possibility of having multiple > wl1251 radios on one system (probably sane assumption from practical > standpoint but still). Basically nvs data are device specific, in ideal case they should be generated in factory by some calibration process (or so). > >> >> Absence of mac address cause that driver generates random mac > >> >> address at every kernel boot which has couple of problems > >> >> (unstable identifier of wireless device due to udev permanent > >> >> storage rules; unpredictable behaviour for dhcp mac address > >> >> assignment, mac address filtering, ...). > >> >> > >> >> Currently there is no way to set (permanent) mac address for > >> >> network interface from userspace. And it does not make sense > >> >> to implement in linux kernel large parser for proprietary > >> >> format of second nand partition where is mac address stored > >> >> only for one device -- Nokia N900. > >> >> > >> >> Driver wl1251.ko loads calibration data via request_firmware() > >> >> for file wl1251-nvs.bin. There are some "example" calibration > >> >> file in linux- firmware repository, but it is not suitable for > >> >> normal usage as real calibration data are per-device specific. > >> > >> You could hook up a script that cooks up the cal/mac file via > >> modprobe's install hook, no? > > > > Via modprobe hook I can either pass custom module parameter or call > > any other system (shell) commands. > > > > As wl1251.ko does not accept mac_address as module parameter, such > > modprobe hook does not help -- as there is absolutely no way from > > userspace to set or change (permanent) mac address. > > Quoting modprobe.d manual: > > install modulename command... > > > > This command instructs modprobe to run your > > command instead of inserting the module in the > > kernel as normal. The command can be any shell > > command: this allows you to do any kind of > > complex processing you might wish. [...] I know. But this do not allow me to send mac address to kernel -- as kernel does not support such command yet (reason for my first question). > You can hook up a script that cooks up wl1251-nvs.bin (caldata, > macaddr) and then insmod the actual wl1251.ko module. Or you can just > cook up the nvs on first device boot and store it in /lib/firmware > (possibly overwriting the "generic" wl1251 from linux-firmware). This is what I would like to prevent -- overwriting (possible readonly) system files with some device specific. It is really bad idea! -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: wl1251 & mac address & calibration data
On Tuesday 22 November 2016 16:22:57 Michal Kazior wrote: > On 21 November 2016 at 16:51, Pali Rohár <pali.ro...@gmail.com> wrote: > > On Friday 11 November 2016 18:20:50 Pali Rohár wrote: > >> Hi! I will open discussion about mac address and calibration data for > >> wl1251 wireless chip again... > >> > >> Problem: Mac address & calibration data for wl1251 chip on Nokia N900 > >> are stored on second nand partition (mtd1) in special proprietary format > >> which is used only for Nokia N900 (probably on N8x0 and N9 too). > >> Wireless driver wl1251.ko cannot work without mac address and > >> calibration data. > > Same problem applies to some ath9k/ath10k supported routers. Some even > carry mac address as implicit offset from ethernet mac address. As far > as I understand OpenWRT cooks cal blobs on first boot prior to loading > modules. So... wl1251 on Nokia N900 is not alone and this problem is there for more drivers and devices. Which means we should come up with some generic solution. > >> Absence of mac address cause that driver generates random mac address at > >> every kernel boot which has couple of problems (unstable identifier of > >> wireless device due to udev permanent storage rules; unpredictable > >> behaviour for dhcp mac address assignment, mac address filtering, ...). > >> > >> Currently there is no way to set (permanent) mac address for network > >> interface from userspace. And it does not make sense to implement in > >> linux kernel large parser for proprietary format of second nand > >> partition where is mac address stored only for one device -- Nokia N900. > >> > >> Driver wl1251.ko loads calibration data via request_firmware() for file > >> wl1251-nvs.bin. There are some "example" calibration file in linux- > >> firmware repository, but it is not suitable for normal usage as real > >> calibration data are per-device specific. > > You could hook up a script that cooks up the cal/mac file via > modprobe's install hook, no? Via modprobe hook I can either pass custom module parameter or call any other system (shell) commands. As wl1251.ko does not accept mac_address as module parameter, such modprobe hook does not help -- as there is absolutely no way from userspace to set or change (permanent) mac address. -- Pali Rohár pali.ro...@gmail.com
Re: wl1251 & mac address & calibration data
On Friday 11 November 2016 18:20:50 Pali Rohár wrote: > Hi! I will open discussion about mac address and calibration data for > wl1251 wireless chip again... > > Problem: Mac address & calibration data for wl1251 chip on Nokia N900 > are stored on second nand partition (mtd1) in special proprietary format > which is used only for Nokia N900 (probably on N8x0 and N9 too). > Wireless driver wl1251.ko cannot work without mac address and > calibration data. > > Absence of mac address cause that driver generates random mac address at > every kernel boot which has couple of problems (unstable identifier of > wireless device due to udev permanent storage rules; unpredictable > behaviour for dhcp mac address assignment, mac address filtering, ...). > > Currently there is no way to set (permanent) mac address for network > interface from userspace. And it does not make sense to implement in > linux kernel large parser for proprietary format of second nand > partition where is mac address stored only for one device -- Nokia N900. > > Driver wl1251.ko loads calibration data via request_firmware() for file > wl1251-nvs.bin. There are some "example" calibration file in linux- > firmware repository, but it is not suitable for normal usage as real > calibration data are per-device specific. > > So questions are: > > 1) How to set mac address from userspace for that wl1251 interface? In > userspace I can write parser for that proprietary format of nand > partition and extract mac address from it Proposed solutions for 1) * Introduce new IOCL for setting that permanent mac address from userspace. Currently we have IOCL for get request * Use request_firmware() (with flag from 2)) to ask for mac address from userspace. This is already used by wl12xx driver (as mac address is part of calibration data firmware file) * Allow to set mac address via sysfs file, e.g. /sys/class/ieee80211/phy0/macaddress > 2) How to send calibration data to wl1251 driver? Those are again stored > in proprietary format and I can write userspace parser for it. Proposed solution for 2) Introduce new flag for request_firmware(), so it first try to use userspace helper for loading firmware file with possibility to fallback to direct VFS access. So... what do you think about it? -- Pali Rohár pali.ro...@gmail.com
wl1251 & mac address & calibration data
Hi! I will open discussion about mac address and calibration data for wl1251 wireless chip again... Problem: Mac address & calibration data for wl1251 chip on Nokia N900 are stored on second nand partition (mtd1) in special proprietary format which is used only for Nokia N900 (probably on N8x0 and N9 too). Wireless driver wl1251.ko cannot work without mac address and calibration data. Absence of mac address cause that driver generates random mac address at every kernel boot which has couple of problems (unstable identifier of wireless device due to udev permanent storage rules; unpredictable behaviour for dhcp mac address assignment, mac address filtering, ...). Currently there is no way to set (permanent) mac address for network interface from userspace. And it does not make sense to implement in linux kernel large parser for proprietary format of second nand partition where is mac address stored only for one device -- Nokia N900. Driver wl1251.ko loads calibration data via request_firmware() for file wl1251-nvs.bin. There are some "example" calibration file in linux- firmware repository, but it is not suitable for normal usage as real calibration data are per-device specific. So questions are: 1) How to set mac address from userspace for that wl1251 interface? In userspace I can write parser for that proprietary format of nand partition and extract mac address from it 2) How to send calibration data to wl1251 driver? Those are again stored in proprietary format and I can write userspace parser for it. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
On Tuesday 14 June 2016 18:40:17 Greg KH wrote: > On Tue, Jun 14, 2016 at 06:28:10PM +0200, Pali Rohár wrote: > > On Saturday 11 June 2016 19:42:26 David Miller wrote: > > > From: Andrew Lunn <and...@lunn.ch> > > > Date: Sat, 11 Jun 2016 17:39:21 +0200 > > > > > > > What is still open is do we want to accept it at all? Do we > > > > accept the concept of putting the same MAC address on multiple > > > > interfaces at hotplug time? Do we trust BIOS vendors to not > > > > keep changing DSDT property name, since it is not > > > > standardised? > > > > > > > > Do we want this at all should be decided by somebody more > > > > senior then those passing comments on the code. > > > > > > Indeed, I think the behavior of using the same MAC address on > > > multiple interfaces if we plug several of these in at once is not > > > good. > > > > > > We shouldn't behave this way just because the Microsoft driver > > > does. > > > > I agree, but in some cases it is night mare for local admins when > > booting different OS cause changing MAC address on local network. > > > > Another similar situation: Imagine that you have two USB network > > cards and both have "burned" into their registers same MAC > > address. If you connect both those USB network cards, linux kernel > > bind appropriate driver which read MAC address for both those > > cards. But those addresses are same. What will linux kernel do in > > this case? > > If you can find such a broken USB device, try it and see :) What do you mean by broken USB device? You have never seen two ethernet cards with same MAC addresses? Right I have not seen two USB, but there is non zero chance that could happen. Specially now when more and more people starts using USB network cards. > (hint, might be hard to find, I've never seen such a device before.) > > I don't see how that pertains to this issue, sorry, how does broken > USB hardware compare to a working Dell device? It is same, how to handle two network cards which tell us, that they have same MAC addresses. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH v6] r8152: Add support for setting pass through MAC address on RTL8153-AD
On Saturday 11 June 2016 19:42:26 David Miller wrote: > From: Andrew Lunn <and...@lunn.ch> > Date: Sat, 11 Jun 2016 17:39:21 +0200 > > > What is still open is do we want to accept it at all? Do we accept > > the concept of putting the same MAC address on multiple interfaces > > at hotplug time? Do we trust BIOS vendors to not keep changing > > DSDT property name, since it is not standardised? > > > > Do we want this at all should be decided by somebody more senior > > then those passing comments on the code. > > Indeed, I think the behavior of using the same MAC address on > multiple interfaces if we plug several of these in at once is not > good. > > We shouldn't behave this way just because the Microsoft driver does. I agree, but in some cases it is night mare for local admins when booting different OS cause changing MAC address on local network. Another similar situation: Imagine that you have two USB network cards and both have "burned" into their registers same MAC address. If you connect both those USB network cards, linux kernel bind appropriate driver which read MAC address for both those cards. But those addresses are same. What will linux kernel do in this case? This is very similar situation as those Dell usb network cards told us "hey, use address which is in ACPI DSDT table". Either we should trust what network card what told us, or not and then generate MAC addresses in better way. Just my opinion... -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH v5] r8152: Add support for setting pass through MAC address on RTL8153-AD
Hi! Another problem which I found now:
On Tuesday 07 June 2016 10:33:47 Mario Limonciello wrote:
> + ret = hex2bin(buf, obj->string.pointer + 9, 6);
> + if (!(ret == 0 && is_valid_ether_addr(buf))) {
> + netif_warn(tp, probe, tp->netdev,
> +"Invalid MAC when reading pass-thru MAC addr: "
> +"%pM\n",
> +buf);
> + goto amacout;
> + }
In case when hex2bin returns zero, but is_valid_ether_addr returns
false, this function returns also zero. And thats wrong, because error
occur.
--
Pali Rohár
pali.ro...@gmail.com
Re: [PATCH v4] r8152: Add support for setting pass through MAC address on RTL8153-AD
a_ocp_read(tp, PLA_IDR, 8, sa.sa_data);
> - else
> - ret = pla_ocp_read(tp, PLA_BACKUP, 8, sa.sa_data);
> + else {
> + /* if this is not an RTL8153-AD, no eFuse mac pass thru set,
> + * or system doesn't provide valid _SB.AMAC this will be
> + * be expected to non-zero
> + */
> + ret = get_vendor_mac_passthru_addr(tp, );
You do not "get" (return) any mac address. Personally I would use "read"
word in function name (like above pla_ocp_read). What about?
ret = vendor_mac_passthru_addr_read(tp, sa.sa_data);
Or something similar... "Get" looks like function "get" something, but
instead it set address in structure.
> + if (ret < 0)
> + ret = pla_ocp_read(tp, PLA_BACKUP, 8, sa.sa_data);
> + }
>
> if (ret < 0) {
> netif_err(tp, probe, dev, "Get ether addr fail\n");
--
Pali Rohár
pali.ro...@gmail.com
Re: [PATCH] r8152: Add support for setting MAC to system's Auxiliary MAC address
ot;first" device. Then you are better off with the > > userspace proposal where you and your users have some chance to > > implement a sensible policy based on e.g. usb port numbers. > > OK, if I can't come up with a way to key on the device being a Dell > dock I'll scrap this entirely kernel approach. E.g. PCI devices have ordinary PCI device & vendor IDs, but have Dell specific subsystem IDs. And via subsystem IDs we can distinguish between Intel graphics card on Dell laptop and on non-Dell laptop. Does not you have some special/modified firmware in those Dell realtek docks (and ability to check from OS some registers)? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH] r8152: Add support for setting MAC to system's Auxiliary MAC address
On Thursday 02 June 2016 20:04:02 Bjørn Mork wrote: > <mario_limoncie...@dell.com> writes: > >> > 2) Track whether this is the first or second USB NIC plugged in. > >> > Only offer it > >> > >> on the first NIC detected by r8152. When the second NIC is > >> plugged in don't match from ACPI. > >> > >> > There would be a question of what to do if the first NIC is > >> > removed and > >> > >> added back if it should get the persistent system MAC or not. > >> > >> > I'd say yes, just make sure that only one NIC can have it at a > >> > time. > >> > >> You are going to get things very complex very quickly if you try > >> to do this. > > > > It's really not that hard, track a module wide static variable > > whether the feature is in use. Track in each device whether the > > feature was in use. If it in use, don't assign the next device > > plugged in via the ACPI string. If a device is removed that has > > the feature activated, change the module wide static variable. > > Having the mac address jump around in an arbitrary way like this is > going to confuse the hell out of your users. Consider what happens > if the user docks a laptop with an r8152 usb dongle already plugged > in... How are you going to explain that the dock gets some other mac > address in this case? How are you going to explain the difference > between using an r8152 based dongle and some other ethernet usb > dongle with your systems? > > Make it behave consistently if you're going to add this. Which can > be done by specifically matching the Dell dock (doesn't it have an > unique Dell device ID?) and ignoring any other r8152 device. You > could also choose to set the same mac for all r8152 devices. Which > is fine, but will probably confuse many users. > > What you definitely should not do is to change the mac for some > arbitrary "first" device. Then you are better off with the userspace > proposal where you and your users have some chance to implement a > sensible policy based on e.g. usb port numbers. This is exactly what I wanted to write, but you were faster :-) You can connect more Dell docks (with r8152 devices) and more non-Dell r8152 devices in random order into Dell laptop. In any case dependent on connect and disconnect order, devices always must have exactly same MAC addresses. Otherwise there will be problems! It confuse users and also admins of networks... So if kernel approach is chosen then I think there are only two solution those satisfy above conditions: First one is: * all non-Dell devices have own MAC address * all Dell devices have (one, same) AUX MAC address Second one is: * all devices (Dell and also non-Dell) have own address * AUX MAC address is never used So what do you (netdev maintainers) think about it? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH] r8152: Add support for setting MAC to system's Auxiliary MAC address
Hi! As ACPI bytecode is untrusted for me and also for running kernel, we
should not expect that it does not contain any bugs or other problems.
So I would propose these checks to prevent something wrong...
On Wednesday 01 June 2016 16:50:44 Mario Limonciello wrote:
> +static void set_auxiliary_addr(struct sockaddr *sa)
> +{
> + acpi_status status;
> + acpi_handle handle;
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *obj;
> + int i;
> + char *ptr;
> +
> + acpi_get_handle(NULL, "\\_SB", );
Check return value of acpi_get_handle
> + status = acpi_evaluate_object(handle, "AMAC", NULL, );
This is question for ACPI devs, it is not possible to call directly?
acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, );
And what happen if we try to evaluate objects which do not exist? Does
not it show some warning or error in dmesg about non existent object?
Such errors should be silent here.
> + obj = (union acpi_object *)buffer.pointer;
Check buffer.type
> + if (ACPI_SUCCESS(status) && (obj->string.length == 0x17)) {
> + /* returns _AUXMAC_#AABBCCDDEEFF#
> + * this pulls out _AUXMAC# from start and # from end
> + */
> + ptr = obj->string.pointer + 9;
Verify that string really contains that _AUXMAX# prefix. This is really
obscure and nonstandard format for specifying MAC address and in my
opinion it should be properly checked. Nonstandard formats can be
changed in future and we could have problems.
> + pr_info("r8152: Using system auxiliary MAC address");
It would be great to write also mac address into that pr_info
> + for (i = 0; i < 6; i++, ptr += 2)
> + sa->sa_data[i] = amac_ascii_to_hex(*ptr) << 4 |
> + amac_ascii_to_hex(*(ptr + 1));
> + }
In case of some acpi check fails throw warning (or error).
And there is memory leak, you allocated buffer with ACPI_ALLOCATE_BUFFER
but you did not free it.
> +}
And my last question is: Are really all Dell docks comes with this one
realtek chip? I'm pessimist in this, because I see how other components
(like HDD vendor, touchpad type, smardcard chips, motherboards, display
panels, wifi chips) can be different in two laptops of same Dell model.
--
Pali Rohár
pali.ro...@gmail.com
Re: AP firmware for TI wl1251 wifi chip (wl1251-fw-ap.bin)
On Sunday 10 April 2016 13:51:41 Pavel Machek wrote: > Is it "hardware can't do AP", "firmware can't do AP" or "current drivers > do not support AP"? We know that current linux TI wl1251.ko driver does not support AP. And I'm still do not know if "hardware can do AP" or not. Also I'm not sure... maybe current firmware can be "forced" for implementing AP mode. David Gnedt implemented packet injection for wl1251.ko via JOIN firmware command. And he wrote me that aircrack utils contains one application which implementing full AP station in userspace just via packet injection radiotap (which is supported by wl1251). So maybe using other firmware (filter) commands together with current packet injection implementation could be possible to create AP mode (usable for standard hostapd daemon)? Just thinking... Anyway, other TI wilink devices have different firmware for AP mode... -- Pali Rohár pali.ro...@gmail.com
Re: AP firmware for TI wl1251 wifi chip (wl1251-fw-ap.bin)
On Wednesday 06 April 2016 13:30:22 Machani, Yaniv wrote: > On Mon, Apr 04, 2016 at 15:39:44, Pali Rohár wrote: > > > In linux-firmware repository [1] is missing AP firmware for TI > > > wl1251 chip. There is only STA firmware wl1251-fw.bin which > > > supports managed and ad-hoc modes. > > > > > > For other TI wilink chips there are -ap.bin firmware files > > > (wl1271-fw-ap.bin and wl128x-fw-ap.bin) which support AP mode. > > > But for > > > wl1251 firmware file with guessed name "wl1251-fw-ap.bin" is > > > missing. > > > > > > Do you have any idea what happened with AP firmware for ti > > > wilink4 wl1251 wifi chip? Or where can be found? Guys from TI, > > > can you help? > > > > > > I see that STA firmware was added into linux-firmware tree in > > > year 2013 by this pull request [2]. > > > > > > [1] - > > > https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmw > > > are .g > > > it/tree/ti-connectivity > > > > > > [2] - > > > http://thread.gmane.org/gmane.linux.kernel/1566500/focus=1571382 > > > > Hi! Anybody has some idea about that AP firmware? > > Hi, > wl1251 does not support AP mode, so there is no firmware for it in > the tree. > > Regards, > Yaniv Hi Yaniv! I read on some TI whitepaper, that wl1251 hardware supports some Soft-AP mode. So I expect that either special FW is needed for it or somehow it is possible to use current released. Do you have any information about it? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: AP firmware for TI wl1251 wifi chip (wl1251-fw-ap.bin)
On Sunday 20 March 2016 00:40:25 Pali Rohár wrote: > Hi! > > In linux-firmware repository [1] is missing AP firmware for TI wl1251 > chip. There is only STA firmware wl1251-fw.bin which supports managed > and ad-hoc modes. > > For other TI wilink chips there are -ap.bin firmware files > (wl1271-fw-ap.bin and wl128x-fw-ap.bin) which support AP mode. But for > wl1251 firmware file with guessed name "wl1251-fw-ap.bin" is missing. > > Do you have any idea what happened with AP firmware for ti wilink4 > wl1251 wifi chip? Or where can be found? Guys from TI, can you help? > > I see that STA firmware was added into linux-firmware tree in year 2013 > by this pull request [2]. > > [1] - > https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/ti-connectivity > > [2] - http://thread.gmane.org/gmane.linux.kernel/1566500/focus=1571382 > Hi! Anybody has some idea about that AP firmware? -- Pali Rohár pali.ro...@gmail.com
Re: AP firmware for TI wl1251 wifi chip (wl1251-fw-ap.bin)
On Monday 21 March 2016 12:35:32 Kalle Valo wrote: > Pali Rohár <pali.ro...@gmail.com> writes: > > > In linux-firmware repository [1] is missing AP firmware for TI wl1251 > > chip. There is only STA firmware wl1251-fw.bin which supports managed > > and ad-hoc modes. > > > > For other TI wilink chips there are -ap.bin firmware files > > (wl1271-fw-ap.bin and wl128x-fw-ap.bin) which support AP mode. But for > > wl1251 firmware file with guessed name "wl1251-fw-ap.bin" is missing. > > > > Do you have any idea what happened with AP firmware for ti wilink4 > > wl1251 wifi chip? Or where can be found? Guys from TI, can you help? > > It's a long time ago but IIRC wl1251 has not ever supported AP mode and > wl1271 was the first one to support it. But I might be wrong of course. Support for AP mode in current kernel driver wl1251.ko is missing, but I could try to write it if there will be firmware for it. According to some TI whitepaper about TI wilink4 devices (wl1251 and wl1253) those devices have support for some Soft-AP mode. -- Pali Rohár pali.ro...@gmail.com
AP firmware for TI wl1251 wifi chip (wl1251-fw-ap.bin)
Hi! In linux-firmware repository [1] is missing AP firmware for TI wl1251 chip. There is only STA firmware wl1251-fw.bin which supports managed and ad-hoc modes. For other TI wilink chips there are -ap.bin firmware files (wl1271-fw-ap.bin and wl128x-fw-ap.bin) which support AP mode. But for wl1251 firmware file with guessed name "wl1251-fw-ap.bin" is missing. Do you have any idea what happened with AP firmware for ti wilink4 wl1251 wifi chip? Or where can be found? Guys from TI, can you help? I see that STA firmware was added into linux-firmware tree in year 2013 by this pull request [2]. [1] - https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/ti-connectivity [2] - http://thread.gmane.org/gmane.linux.kernel/1566500/focus=1571382 -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH v2] wl1251: add sysfs interface for bluetooth coexistence mode configuration
On Thursday 21 January 2016 16:44:33 Kalle Valo wrote: > Pali Rohár <pali.ro...@gmail.com> writes: > > > On Thursday 21 January 2016 15:48:14 Kalle Valo wrote: > >> Pali Rohár <pali.ro...@gmail.com> writes: > >> > >> > On Thursday 14 January 2016 10:16:54 Pavel Machek wrote: > >> >> On Wed 2016-01-13 23:32:47, Arend van Spriel wrote: > >> >> > On 12/26/2015 12:45 PM, Pali Rohár wrote: > >> >> > >Port the bt_coex_mode sysfs interface from wl1251 driver version > >> >> > >included > >> >> > >in the Maemo Fremantle kernel to allow bt-coexistence mode > >> >> > >configuration. > >> >> > >This enables userspace applications to set one of the modes > >> >> > >WL1251_BT_COEX_OFF, WL1251_BT_COEX_ENABLE and > >> >> > >WL1251_BT_COEX_MONOAUDIO. > >> >> > >The default mode is WL1251_BT_COEX_OFF. > >> >> > >It should be noted that this driver always enabled bt-coexistence > >> >> > >before > >> >> > >and enabled bt-coexistence directly affects the receiving > >> >> > >performance, > >> >> > >rendering it unusable in some low-signal situations. Especially > >> >> > >monitor > >> >> > >mode is affected very badly with bt-coexistence enabled. > >> >> > > >> >> > So what user-space process will be using this interface. Did you > >> >> > consider > >> >> > adding debugfs interface? In case of monitor mode you could consider > >> >> > disabling bt-coex from within the driver itself. > >> >> > >> >> This aint no debugging feature. > >> > > >> > Right, bt-coex is not for debugging purpose, but for normal usage, when > >> > user want to use together bluetooth and wifi or just one of those. > >> > >> I think most of other drivers have a debugfs interface for btcoex, I > >> guess mostly for testing purposes. But this really should be added to > >> cfg80211. > > > > All other TI wireless drivers have "bt_coex_state" sysfs node. > > Then that's a mistake, they shouldn't have that. But it is there, wl1251 is also TI wireless driver and for last two years there is no interface to deal with this problem... So as other drivers do, I'm proposing solution which fix bt coex also for wl1251 driver need on Nokia N900. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH v2] wl1251: add sysfs interface for bluetooth coexistence mode configuration
On Thursday 21 January 2016 15:48:14 Kalle Valo wrote: > Pali Rohár <pali.ro...@gmail.com> writes: > > > On Thursday 14 January 2016 10:16:54 Pavel Machek wrote: > >> On Wed 2016-01-13 23:32:47, Arend van Spriel wrote: > >> > On 12/26/2015 12:45 PM, Pali Rohár wrote: > >> > >Port the bt_coex_mode sysfs interface from wl1251 driver version > >> > >included > >> > >in the Maemo Fremantle kernel to allow bt-coexistence mode > >> > >configuration. > >> > >This enables userspace applications to set one of the modes > >> > >WL1251_BT_COEX_OFF, WL1251_BT_COEX_ENABLE and WL1251_BT_COEX_MONOAUDIO. > >> > >The default mode is WL1251_BT_COEX_OFF. > >> > >It should be noted that this driver always enabled bt-coexistence before > >> > >and enabled bt-coexistence directly affects the receiving performance, > >> > >rendering it unusable in some low-signal situations. Especially monitor > >> > >mode is affected very badly with bt-coexistence enabled. > >> > > >> > So what user-space process will be using this interface. Did you consider > >> > adding debugfs interface? In case of monitor mode you could consider > >> > disabling bt-coex from within the driver itself. > >> > >> This aint no debugging feature. > > > > Right, bt-coex is not for debugging purpose, but for normal usage, when > > user want to use together bluetooth and wifi or just one of those. > > I think most of other drivers have a debugfs interface for btcoex, I > guess mostly for testing purposes. But this really should be added to > cfg80211. All other TI wireless drivers have "bt_coex_state" sysfs node. -- Pali Rohár pali.ro...@gmail.com
Re: [PATCH v2] wl1251: add sysfs interface for bluetooth coexistence mode configuration
On Thursday 14 January 2016 10:16:54 Pavel Machek wrote: > On Wed 2016-01-13 23:32:47, Arend van Spriel wrote: > > On 12/26/2015 12:45 PM, Pali Rohár wrote: > > >Port the bt_coex_mode sysfs interface from wl1251 driver version included > > >in the Maemo Fremantle kernel to allow bt-coexistence mode configuration. > > >This enables userspace applications to set one of the modes > > >WL1251_BT_COEX_OFF, WL1251_BT_COEX_ENABLE and WL1251_BT_COEX_MONOAUDIO. > > >The default mode is WL1251_BT_COEX_OFF. > > >It should be noted that this driver always enabled bt-coexistence before > > >and enabled bt-coexistence directly affects the receiving performance, > > >rendering it unusable in some low-signal situations. Especially monitor > > >mode is affected very badly with bt-coexistence enabled. > > > > So what user-space process will be using this interface. Did you consider > > adding debugfs interface? In case of monitor mode you could consider > > disabling bt-coex from within the driver itself. > > This aint no debugging feature. > Pavel Right, bt-coex is not for debugging purpose, but for normal usage, when user want to use together bluetooth and wifi or just one of those. Are there any other objections for this patch? -- Pali Rohár pali.ro...@gmail.com
[PATCH v2] wl1251: add sysfs interface for bluetooth coexistence mode configuration
Port the bt_coex_mode sysfs interface from wl1251 driver version included
in the Maemo Fremantle kernel to allow bt-coexistence mode configuration.
This enables userspace applications to set one of the modes
WL1251_BT_COEX_OFF, WL1251_BT_COEX_ENABLE and WL1251_BT_COEX_MONOAUDIO.
The default mode is WL1251_BT_COEX_OFF.
It should be noted that this driver always enabled bt-coexistence before
and enabled bt-coexistence directly affects the receiving performance,
rendering it unusable in some low-signal situations. Especially monitor
mode is affected very badly with bt-coexistence enabled.
Signed-off-by: David Gnedt <david.gn...@davizone.at>
Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
I'm resending this patch for review again as after two years there is no
nl80211 interface for bt coex and wl1251 on Nokia N900 needs it. Once
there will be common interface for bt coex I can rewrite my patches, but
I do not want to wait another 2 years...
Changes:
In v2 is sysfs node attached directly to wl1251 device instead of creating
new platform device for sysfs node. So sysfs node is now available at:
/sys/class/net/wlan0/device/bt_coex_mode
---
drivers/net/wireless/ti/wl1251/acx.c| 43 ++--
drivers/net/wireless/ti/wl1251/acx.h|8 +--
drivers/net/wireless/ti/wl1251/init.c |6 +--
drivers/net/wireless/ti/wl1251/main.c | 84 +++
drivers/net/wireless/ti/wl1251/wl1251.h |8 +++
5 files changed, 137 insertions(+), 12 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/acx.c
b/drivers/net/wireless/ti/wl1251/acx.c
index d6fbdda..a119d77 100644
--- a/drivers/net/wireless/ti/wl1251/acx.c
+++ b/drivers/net/wireless/ti/wl1251/acx.c
@@ -539,7 +539,7 @@ out:
return ret;
}
-int wl1251_acx_sg_enable(struct wl1251 *wl)
+int wl1251_acx_sg_enable(struct wl1251 *wl, u8 mode)
{
struct acx_bt_wlan_coex *pta;
int ret;
@@ -550,7 +550,7 @@ int wl1251_acx_sg_enable(struct wl1251 *wl)
if (!pta)
return -ENOMEM;
- pta->enable = SG_ENABLE;
+ pta->enable = mode;
ret = wl1251_cmd_configure(wl, ACX_SG_ENABLE, pta, sizeof(*pta));
if (ret < 0) {
@@ -563,7 +563,7 @@ out:
return ret;
}
-int wl1251_acx_sg_cfg(struct wl1251 *wl)
+int wl1251_acx_sg_cfg(struct wl1251 *wl, u16 wake_up_beacon)
{
struct acx_bt_wlan_coex_param *param;
int ret;
@@ -586,7 +586,7 @@ int wl1251_acx_sg_cfg(struct wl1251 *wl)
param->wlan_cycle_fast = PTA_CYCLE_TIME_FAST_DEF;
param->bt_anti_starvation_period = PTA_ANTI_STARVE_PERIOD_DEF;
param->next_bt_lp_packet = PTA_TIMEOUT_NEXT_BT_LP_PACKET_DEF;
- param->wake_up_beacon = PTA_TIME_BEFORE_BEACON_DEF;
+ param->wake_up_beacon = wake_up_beacon;
param->hp_dm_max_guard_time = PTA_HPDM_MAX_TIME_DEF;
param->next_wlan_packet = PTA_TIME_OUT_NEXT_WLAN_DEF;
param->antenna_type = PTA_ANTENNA_TYPE_DEF;
@@ -615,6 +615,41 @@ out:
return ret;
}
+int wl1251_acx_sg_configure(struct wl1251 *wl, bool force)
+{
+ int ret;
+
+ if (wl->state == WL1251_STATE_OFF && !force)
+ return 0;
+
+ switch (wl->bt_coex_mode) {
+ case WL1251_BT_COEX_OFF:
+ ret = wl1251_acx_sg_enable(wl, SG_DISABLE);
+ if (ret)
+ break;
+ ret = wl1251_acx_sg_cfg(wl, 0);
+ break;
+ case WL1251_BT_COEX_ENABLE:
+ ret = wl1251_acx_sg_enable(wl, SG_ENABLE);
+ if (ret)
+ break;
+ ret = wl1251_acx_sg_cfg(wl, PTA_TIME_BEFORE_BEACON_DEF);
+ break;
+ case WL1251_BT_COEX_MONOAUDIO:
+ ret = wl1251_acx_sg_enable(wl, SG_ENABLE);
+ if (ret)
+ break;
+ ret = wl1251_acx_sg_cfg(wl, PTA_TIME_BEFORE_BEACON_MONO_AUDIO);
+ break;
+ default:
+ wl1251_error("Invalid BT co-ex mode!");
+ ret = -EOPNOTSUPP;
+ break;
+ }
+
+ return ret;
+}
+
int wl1251_acx_cca_threshold(struct wl1251 *wl)
{
struct acx_energy_detection *detection;
diff --git a/drivers/net/wireless/ti/wl1251/acx.h
b/drivers/net/wireless/ti/wl1251/acx.h
index 2bdec38..820573c 100644
--- a/drivers/net/wireless/ti/wl1251/acx.h
+++ b/drivers/net/wireless/ti/wl1251/acx.h
@@ -558,7 +558,8 @@ struct acx_bt_wlan_coex {
#define PTA_ANTI_STARVE_PERIOD_DEF (500)
#define PTA_ANTI_STARVE_NUM_CYCLE_DEF(4)
#define PTA_ALLOW_PA_SD_DEF (1)
-#define PTA_TIME_BEFORE_BEACON_DEF (6300)
+#define PTA_TIME_BEFORE_BEACON_DEF (500)
+#define PTA_TIME_BEFORE_BEACON_MONO_AUDIO (6300)
#define PTA_HPDM_MAX_TIME_DEF(1600)
#define PTA_TIME_OUT_NEXT_WLAN_DEF (2550)
#define PTA_AUTO_MODE_NO_CTS_DEF (0)
@@ -1470,8 +1471,9 @@ int
