* Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
From: Pavel Machek @ 2016-12-26 15:43 UTC (permalink / raw)
To: Pali Rohár
Cc: Arend Van Spriel, Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman,
Kalle Valo, David Gnedt, Michal Kazior, Daniel Wagner,
Tony Lindgren, Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <201612252146.44496@pali>
[-- Attachment #1: Type: text/plain, Size: 2621 bytes --]
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.
> > 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.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [PATCH] mac80211: Fix addition of mesh configuration element
From: Ilan Peer @ 2016-12-26 16:17 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, masashi.honma, Ilan Peer
The code was setting the capabilities byte to zero,
after it was already properly set previously. Fix it.
The bug was found while debugging hwsim mesh tests failures
that happened in commit 76f43b4 (mac80211: Remove invalid flag
operations in mesh TSF synchronization).
Signed-off-by: Ilan Peer <ilan.peer@intel.com>
---
net/mac80211/mesh.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index cc2a63b..9c23172 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -279,8 +279,6 @@ int mesh_add_meshconf_ie(struct ieee80211_sub_if_data *sdata,
/* Mesh PS mode. See IEEE802.11-2012 8.4.2.100.8 */
*pos |= ifmsh->ps_peers_deep_sleep ?
IEEE80211_MESHCONF_CAPAB_POWER_SAVE_LEVEL : 0x00;
- *pos++ = 0x00;
-
return 0;
}
--
1.9.1
^ permalink raw reply related
* RE: ath10k firmware sends probes on DFS channels without radar detection
From: Jean-Pierre Tosoni @ 2016-12-26 11:15 UTC (permalink / raw)
To: 'Jouni Malinen'; +Cc: 'Ben Greear', linux-wireless, ath10k
In-Reply-To: <20161215225827.GB24883@w1.fi>
> -----Message d'origine-----
> De : Jouni Malinen [mailto:j@w1.fi]
> Envoyé : jeudi 15 décembre 2016 23:58
> À : Jean-Pierre Tosoni
> Cc : 'Ben Greear'; linux-wireless@vger.kernel.org;
> ath10k@lists.infradead.org
> Objet : Re: ath10k firmware sends probes on DFS channels without radar
> detection
>
> On Thu, Dec 15, 2016 at 06:53:47PM +0100, Jean-Pierre Tosoni wrote:
> > > > Thanks for the suggestion, I already tried something like this in
> > > > wmi.c, with the same result:
> > > >
> > > > - Before patching the firmware scans DFS channels actively (with
> > > probes).
> > > >
> > > > - After patching, the firmware scans DFS channels passively
> > > > *until* any beacon is received on the DFS channel. When *any*
> > > > beacon is seen, the firmware decides to scan actively on its own,
> > > > without any new IR/RADAR info from the driver.
> > > >
> > > > So, your patch is required but not sufficient.
> > > >
> > > > Somehow I was able to overcome this by reloading the regulation
> > > > domain in the radio card before each scan request:
>
> Interesting.. I'm not completely sure what could have changed the behavior
> based on beacon hint. I thought it was cfg80211, but if the simple change
> for doing NO_IR | RADAR is not sufficient, it would seem to imply that
> something else can do this. Some more debugging to do, I guess.
After some debugging I think the card firmware does this, probably due to
the lack of precise definition of NO_IR, see below.
>
> > The distinction between NO_IR and CHAN_RADAR is not very clear to me.
> > NO_IR appears only in the world regulatory domain so it's not relevant
> here.
>
> NO_IR is a combination of not allowing AP, IBSS, or active scanning
> without having somehow been enabled by another device. RADAR has that same
> impact and in addition, requirement for doing radar detection and DFS by a
> master device.
Ah, thanks. But then, NO_IR does not define the way for the "other device"
to enable the local device? So, depending on the interpretation, it can
render the local device unusable. OTOH RADAR defines a way which depends on
the local regulations.
>
> > I'd say
> > "the CHAN_RADAR flag should always make the firmware never do IR when
> > probing"
> > ...maybe, except if the channel is the operating channel. (this should
> > exclude unassociated stations)
>
> For most cases, I'd agree that active scanning should not be used on DFS
> channels. That said, unicast Probe Request frame to the current AP while
> associated could be a reasonable exception. In addition, WPS with PBC
> depends on Probe Request frames to allow PBC session overlap detection, so
> there might be sufficient justification to allow Probe Request frame to be
> sent out for a very short duration (couple of seconds) after seeing a
> Beacon frame on the channel for such special cases.
I agree that unicast probes to the current AP should go through. It goes
with my condition "operating channel".
For WPS, I do not know it well, but I guess probes are acceptable if
1) they are not sent repeatedly over a long period of time during
unassociated state,
2) the AP uses CAC.
And here, both seem to be true.
>
> --
> Jouni Malinen PGP id EFC895FA
Regards, Jean-Pierre
^ permalink raw reply
* Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
From: Pali Rohár @ 2016-12-25 20:46 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <e728586e-80bf-c9e0-0063-71da4c4aba85@broadcom.com>
[-- Attachment #1: Type: Text/Plain, Size: 5551 bytes --]
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.rohar@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(&fw, WL1251_NVS_NAME, dev);
> > + ret = request_firmware_prefer_user(&fw, WL1251_NVS_NAME, dev);
> >
> > if (ret < 0) {
> >
> > wl1251_error("could not get nvs file: %d", ret);
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
From: Arend Van Spriel @ 2016-12-25 20:15 UTC (permalink / raw)
To: Pali Rohár, Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman,
Kalle Valo, David Gnedt, Michal Kazior, Daniel Wagner,
Tony Lindgren, Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov,
Aaro Koskinen, Grazvydas Ignotas
Cc: linux-kernel, linux-wireless, netdev
In-Reply-To: <1482598381-16513-3-git-send-email-pali.rohar@gmail.com>
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?
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. You should consider other device
configurations. Not just N900.
Regards,
Arend
> Signed-off-by: Pali Rohár <pali.rohar@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(&fw, WL1251_NVS_NAME, dev);
> + ret = request_firmware_prefer_user(&fw, WL1251_NVS_NAME, dev);
>
> if (ret < 0) {
> wl1251_error("could not get nvs file: %d", ret);
>
^ permalink raw reply
* [PATCH 1/2] nfc: Add support RC-S380P to port100
From: OGAWA Hirofumi @ 2016-12-25 9:00 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Aloisio Almeida Jr, Lauro Ramos Venancio, linux-wireless,
Andrew Morton
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
drivers/nfc/port100.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff -puN drivers/nfc/port100.c~nfc-add-rcs380p drivers/nfc/port100.c
--- linux/drivers/nfc/port100.c~nfc-add-rcs380p 2016-12-18 22:16:53.503673411 +0900
+++ linux-hirofumi/drivers/nfc/port100.c 2016-12-18 22:16:53.504673416 +0900
@@ -21,8 +21,9 @@
#define VERSION "0.1"
-#define SONY_VENDOR_ID 0x054c
-#define RCS380_PRODUCT_ID 0x06c1
+#define SONY_VENDOR_ID 0x054c
+#define RCS380S_PRODUCT_ID 0x06c1
+#define RCS380P_PRODUCT_ID 0x06c3
#define PORT100_PROTOCOLS (NFC_PROTO_JEWEL_MASK | \
NFC_PROTO_MIFARE_MASK | \
@@ -1477,7 +1478,8 @@ static struct nfc_digital_ops port100_di
};
static const struct usb_device_id port100_table[] = {
- { USB_DEVICE(SONY_VENDOR_ID, RCS380_PRODUCT_ID), },
+ { USB_DEVICE(SONY_VENDOR_ID, RCS380S_PRODUCT_ID), },
+ { USB_DEVICE(SONY_VENDOR_ID, RCS380P_PRODUCT_ID), },
{ }
};
MODULE_DEVICE_TABLE(usb, port100_table);
_
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* [PATCH 2/2] nfc: Send same info for both of NFC_CMD_GET_DEVICE and NFC_EVENT_DEVICE_ADDED
From: OGAWA Hirofumi @ 2016-12-25 9:01 UTC (permalink / raw)
To: Samuel Ortiz
Cc: Aloisio Almeida Jr, Lauro Ramos Venancio, linux-wireless,
Andrew Morton
In-Reply-To: <87tw9s9ydi.fsf@mail.parknet.co.jp>
Now, NFC_EVENT_DEVICE_ADDED doesn't send NFC_ATTR_RF_MODE. But
NFC_CMD_GET_DEVICE send.
To get NFC_ATTR_RF_MODE, we have to call NFC_CMD_GET_DEVICE just for
NFC_ATTR_RF_MODE when get NFC_EVENT_DEVICE_ADDED.
This fixes those inconsistent.
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---
net/nfc/netlink.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff -puN net/nfc/netlink.c~nfc-send-same-info net/nfc/netlink.c
--- linux/net/nfc/netlink.c~nfc-send-same-info 2016-12-18 22:16:55.805686290 +0900
+++ linux-hirofumi/net/nfc/netlink.c 2016-12-18 22:16:55.806686296 +0900
@@ -311,6 +311,17 @@ free_msg:
return -EMSGSIZE;
}
+static int nfc_genl_setup_device_added(struct nfc_dev *dev, struct sk_buff *msg)
+{
+ if (nla_put_string(msg, NFC_ATTR_DEVICE_NAME, nfc_device_name(dev)) ||
+ nla_put_u32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx) ||
+ nla_put_u32(msg, NFC_ATTR_PROTOCOLS, dev->supported_protocols) ||
+ nla_put_u8(msg, NFC_ATTR_DEVICE_POWERED, dev->dev_up) ||
+ nla_put_u8(msg, NFC_ATTR_RF_MODE, dev->rf_mode))
+ return -1;
+ return 0;
+}
+
int nfc_genl_device_added(struct nfc_dev *dev)
{
struct sk_buff *msg;
@@ -325,10 +336,7 @@ int nfc_genl_device_added(struct nfc_dev
if (!hdr)
goto free_msg;
- if (nla_put_string(msg, NFC_ATTR_DEVICE_NAME, nfc_device_name(dev)) ||
- nla_put_u32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx) ||
- nla_put_u32(msg, NFC_ATTR_PROTOCOLS, dev->supported_protocols) ||
- nla_put_u8(msg, NFC_ATTR_DEVICE_POWERED, dev->dev_up))
+ if (nfc_genl_setup_device_added(dev, msg))
goto nla_put_failure;
genlmsg_end(msg, hdr);
@@ -604,11 +612,7 @@ static int nfc_genl_send_device(struct s
if (cb)
genl_dump_check_consistent(cb, hdr, &nfc_genl_family);
- if (nla_put_string(msg, NFC_ATTR_DEVICE_NAME, nfc_device_name(dev)) ||
- nla_put_u32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx) ||
- nla_put_u32(msg, NFC_ATTR_PROTOCOLS, dev->supported_protocols) ||
- nla_put_u8(msg, NFC_ATTR_DEVICE_POWERED, dev->dev_up) ||
- nla_put_u8(msg, NFC_ATTR_RF_MODE, dev->rf_mode))
+ if (nfc_genl_setup_device_added(dev, msg))
goto nla_put_failure;
genlmsg_end(msg, hdr);
_
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
^ permalink raw reply
* Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data
From: Pali Rohár @ 2016-12-24 18:44 UTC (permalink / raw)
To: Pavel Machek
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <20161224181730.GD13590@amd>
[-- Attachment #1: Type: Text/Plain, Size: 1277 bytes --]
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.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 5/6] wl1251: Parse and use MAC address from supplied NVS data
From: Pali Rohár @ 2016-12-24 18:40 UTC (permalink / raw)
To: Pavel Machek
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <20161224181421.GC13590@amd>
[-- Attachment #1: Type: Text/Plain, Size: 663 bytes --]
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&paste 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.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid
From: Pali Rohár @ 2016-12-24 18:38 UTC (permalink / raw)
To: Pavel Machek
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <20161224180854.GB13590@amd>
[-- Attachment #1: Type: Text/Plain, Size: 1251 bytes --]
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.rohar@gmail.com>
>
> Acked-by: Pavel Machek <pavel@ucw.cz>
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data
From: Pavel Machek @ 2016-12-24 18:17 UTC (permalink / raw)
To: Pali Rohár
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <1482598381-16513-7-git-send-email-pali.rohar@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 960 bytes --]
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.
> + 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?
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 5/6] wl1251: Parse and use MAC address from supplied NVS data
From: Pavel Machek @ 2016-12-24 18:14 UTC (permalink / raw)
To: Pali Rohár
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <1482598381-16513-6-git-send-email-pali.rohar@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3501 bytes --]
On Sat 2016-12-24 17:53:00, Pali Rohár wrote:
> 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.
will be used instead of randomly generated one.
> This patch also move code for requesting NVS data from userspace to driver
"moves"
> initialization code to make sure that NVS data will be there at time when
> permanent MAC address is needed.
"at a time"
> Calibration NVS data for wl1251 are model specific. Every one device with
"device specific"? "Every device".
> 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
"are found"
> contains MAC address 00:00:20:07:03:09. So this MAC address is marked as
"contain"
> 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.rohar@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 */
remove "default".
> + 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;
> + }
Is goto out here good idea? IMNSHO it is copy&paste bug, it should
just proceed with generating random address.
> 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;
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid
From: Pavel Machek @ 2016-12-24 18:08 UTC (permalink / raw)
To: Pali Rohár
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <1482598381-16513-5-git-send-email-pali.rohar@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 991 bytes --]
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?
The only case where this helps is if wl1251_read_eeprom_mac() succeeds
but reads invalid address.
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 3/6] wl1251: Update wl->nvs_len after wl->nvs is valid
From: Pavel Machek @ 2016-12-24 18:02 UTC (permalink / raw)
To: Pali Rohár
Cc: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas, linux-kernel, linux-wireless, netdev
In-Reply-To: <1482598381-16513-4-git-send-email-pali.rohar@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]
On Sat 2016-12-24 17:52:58, Pali Rohár wrote:
> In case kmemdup fails thne wl->nvs_len will contains invalid non-zero size.
> This patch fixes it.
If kmemdup fails, then wl->nvs_len will contain invalid non-zero size.
?
This probably should go as first in series, as it is bugfix?
Pavel
Acked-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Pali Rohár <pali.rohar@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:
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Mark Greer @ 2016-12-24 17:24 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
linux-kernel, Justin Bronder, Jaret Cantu
In-Reply-To: <CAO7Z3W+n85jbr8cmj3HJV1c+eL=oRh-XRy+O=iZ+mD=ov2WABA@mail.gmail.com>
On Sat, Dec 24, 2016 at 11:17:18AM -0500, Geoff Lansberry wrote:
> Mark - I'm sorry, but I did not write this code, and therefore was not
> able to accurately describe it. It is fixing a different issue, not
> the neard segfault that we are still chasing. Last week Jaret Cantu
> sent a separate email explaining the purpose of the code, which had
> you copied, did you see that?
Hm, no, I didn't. I received an email from Justin Bronder but not from
Jaret Cantu. Justin's email did help but is still pretty high-level.
We need a clear understanding as to what is happening in the digital
layer and the driver to know how execution is getting into a block of
error handling code that should never be executed. Once we understand
that we can start thinking about what the best fix is.
> Does it explain why it was done to
> your satisfaction? I've asked him to join in on the effort to push
> the change upstream, however he will not be available until the new
> year.
I expect that it would help if he joins. After the holidays is fine -
I think many people are taking it easy for the next week or so, anyway.
> I know you did suggest that we split off that change from the others,
> and if now is the time to do that, let me know. If you don't have
> the email from Jaret, also please let me know and I will forward it to
> you.
I think it would help you if you split it off because the first two patches
have a good chance of being accepted but this one doesn't (yet). If you
separate the them, it will make it easier for Samuel to take the first two
(or he may take the first two anyway but its always good to make it as
easy maintainers as you can).
Mark
--
^ permalink raw reply
* [PATCH 6/6] wl1251: Set generated MAC address back to NVS data
From: Pali Rohár @ 2016-12-24 16:53 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel, linux-wireless, netdev, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar@gmail.com>
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.rohar@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
^ permalink raw reply related
* [PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid
From: Pali Rohár @ 2016-12-24 16:52 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel, linux-wireless, netdev, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar@gmail.com>
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.rohar@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), &wl1251_ops);
if (!hw) {
@@ -1674,13 +1690,6 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
INIT_WORK(&wl->irq_work, wl1251_irq_work);
INIT_WORK(&wl->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(&wl->mutex);
--
1.7.9.5
^ permalink raw reply related
* [PATCH 5/6] wl1251: Parse and use MAC address from supplied NVS data
From: Pali Rohár @ 2016-12-24 16:53 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel, linux-wireless, netdev, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar@gmail.com>
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.rohar@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
^ permalink raw reply related
* [PATCH 1/6] firmware: Add request_firmware_prefer_user() function
From: Pali Rohár @ 2016-12-24 16:52 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel, linux-wireless, netdev, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar@gmail.com>
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.rohar@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
^ permalink raw reply related
* [PATCH 3/6] wl1251: Update wl->nvs_len after wl->nvs is valid
From: Pali Rohár @ 2016-12-24 16:52 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel, linux-wireless, netdev, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar@gmail.com>
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.rohar@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
^ permalink raw reply related
* [PATCH 0/6] wl1251: Fix MAC address for Nokia N900
From: Pali Rohár @ 2016-12-24 16:52 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel, linux-wireless, netdev, Pali Rohár
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
^ permalink raw reply
* [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
From: Pali Rohár @ 2016-12-24 16:52 UTC (permalink / raw)
To: Ming Lei, Luis R. Rodriguez, Greg Kroah-Hartman, Kalle Valo,
David Gnedt, Michal Kazior, Daniel Wagner, Tony Lindgren,
Sebastian Reichel, Pavel Machek, Ivaylo Dimitrov, Aaro Koskinen,
Grazvydas Ignotas
Cc: linux-kernel, linux-wireless, netdev, Pali Rohár
In-Reply-To: <1482598381-16513-1-git-send-email-pali.rohar@gmail.com>
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.rohar@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(&fw, WL1251_NVS_NAME, dev);
+ ret = request_firmware_prefer_user(&fw, WL1251_NVS_NAME, dev);
if (ret < 0) {
wl1251_error("could not get nvs file: %d", ret);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Geoff Lansberry @ 2016-12-24 16:17 UTC (permalink / raw)
To: Mark Greer
Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
Samuel Ortiz, robh+dt, mark.rutland, netdev, devicetree,
linux-kernel, Justin Bronder, Jaret Cantu
In-Reply-To: <20161224060141.GA9069@animalcreek.com>
Mark - I'm sorry, but I did not write this code, and therefore was not
able to accurately describe it. It is fixing a different issue, not
the neard segfault that we are still chasing. Last week Jaret Cantu
sent a separate email explaining the purpose of the code, which had
you copied, did you see that? Does it explain why it was done to
your satisfaction? I've asked him to join in on the effort to push
the change upstream, however he will not be available until the new
year.
I know you did suggest that we split off that change from the others,
and if now is the time to do that, let me know. If you don't have
the email from Jaret, also please let me know and I will forward it to
you.
Geoff
Geoff Lansberry
Engineering Guy
Kuv=C3=A9e, Inc
125 Kingston St., 3rd Floor
Boston, MA 02111
1-617-290-1118 (m)
geoff.lansberry (skype)
http://www.kuvee.com
On Sat, Dec 24, 2016 at 1:01 AM, Mark Greer <mgreer@animalcreek.com> wrote:
> On Wed, Dec 21, 2016 at 11:18:34PM -0500, Geoff Lansberry wrote:
>> From: Jaret Cantu <jaret.cantu@timesys.com>
>>
>> Repeated polling attempts cause a NULL dereference error to occur.
>> This is because the state of the trf7970a is currently reading but
>> another request has been made to send a command before it has finished.
>>
>> The solution is to properly kill the waiting reading (workqueue)
>> before failing on the send.
>>
>> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
>> ---
>
> You've still provided virtually no information on the actual problem(s)
> nor justified why you think this is the best solution. You're adding
> code to a section of code that should _never_ be executed so the only
> reasonable things I can infer is that there are, at least, two problems:
>
> 1) There is a bug causing execution to get into this block of code.
>
> 2) Once in this block of code, there is another bug.
>
> You seem to be attempting to fix 2) and completely ignoring 1).
> 1) is the first bug that needs to be root-caused and fixed.
>
> Also, what exactly is the "NULL dereference error" you mention?
> Is this the neard crash you talked about in another thread or is
> this a kernel crash? If it is the kernel crash, please post the
> relevant information. If this is the neard crash - which seems
> unlikely - then how can changing a section of kernel code that
> shouldn't be executed in the first place fix that?
>
> Mark
> --
^ permalink raw reply
* Re: wl1251 NVS calibration data format
From: Pali Rohár @ 2016-12-24 12:57 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Gery Kahn, Shahar Lev, Kalle Valo, linux-wireless, linux-kernel,
Pavel Machek, Ivaylo Dimitrov
In-Reply-To: <201612171214.50820@pali>
[-- Attachment #1: Type: Text/Plain, Size: 921 bytes --]
On Saturday 17 December 2016 12:14:50 Pali Rohár wrote:
> I will try to play with driver if it is really truth!
>
> I already looked into original TI's multiplatform HAL driver for
> wl1251 chip (big mess) and found there that there is wl1251 command
> to read mac address from chip. It could be done by this wl1251
> function:
>
> wl1251_cmd_interrogate(wl, DOT11_STATION_ID, mac, sizeof(*mac))
>
> (same id as for setting permanent mac address, but opposite to read
> it)
Confirmed! Calling that function (before setting real linux mac address)
returns MAC address 00:00:20:07:03:09.
Changing NVS data at position 0x1c-0x21 changes also what above function
returns.
So really at position 0x1c-0x21 in NVS data is stored MAC address (in
reverse order). Just default is some unknown 00:00:20:07:03:09.
So MAC address is really part of NVS data.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Mark Greer @ 2016-12-24 6:01 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo, robh+dt,
mark.rutland, netdev, devicetree, linux-kernel, justin,
Jaret Cantu
In-Reply-To: <1482380314-16440-3-git-send-email-geoff@kuvee.com>
On Wed, Dec 21, 2016 at 11:18:34PM -0500, Geoff Lansberry wrote:
> From: Jaret Cantu <jaret.cantu@timesys.com>
>
> Repeated polling attempts cause a NULL dereference error to occur.
> This is because the state of the trf7970a is currently reading but
> another request has been made to send a command before it has finished.
>
> The solution is to properly kill the waiting reading (workqueue)
> before failing on the send.
>
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
> ---
You've still provided virtually no information on the actual problem(s)
nor justified why you think this is the best solution. You're adding
code to a section of code that should _never_ be executed so the only
reasonable things I can infer is that there are, at least, two problems:
1) There is a bug causing execution to get into this block of code.
2) Once in this block of code, there is another bug.
You seem to be attempting to fix 2) and completely ignoring 1).
1) is the first bug that needs to be root-caused and fixed.
Also, what exactly is the "NULL dereference error" you mention?
Is this the neard crash you talked about in another thread or is
this a kernel crash? If it is the kernel crash, please post the
relevant information. If this is the neard crash - which seems
unlikely - then how can changing a section of kernel code that
shouldn't be executed in the first place fix that?
Mark
--
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox