* Re: wl1251 & mac address & calibration data
From: Pali Rohár @ 2016-12-16 10:26 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Kalle Valo, Sebastian Reichel, Pavel Machek, Michal Kazior,
Ivaylo Dimitrov, Aaro Koskinen, Tony Lindgren, linux-wireless,
Network Development, linux-kernel, Luis R. Rodriguez
In-Reply-To: <76365770-f5ba-9565-3fca-710518f64d81@broadcom.com>
[-- Attachment #1: Type: Text/Plain, Size: 3509 bytes --]
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 <kvalo@codeaurora.org> wrote:
> >> (Adding Luis because he has been working on request_firmware()
> >> lately)
> >>
> >> Pali Rohár <pali.rohar@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.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [RFC v2 06/11] ath10k: htc: Added ATH10K_HTC_FLAG_BUNDLE_LSB
From: Valo, Kalle @ 2016-12-16 10:23 UTC (permalink / raw)
To: Erik Stromdahl; +Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
In-Reply-To: <1479496971-19174-7-git-send-email-erik.stromdahl@gmail.com>
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
> ---
> drivers/net/wireless/ath/ath10k/htc.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless=
/ath/ath10k/htc.h
> index f94b25a..2963694 100644
> --- a/drivers/net/wireless/ath/ath10k/htc.h
> +++ b/drivers/net/wireless/ath/ath10k/htc.h
> @@ -62,6 +62,8 @@ enum ath10k_htc_rx_flags {
> ATH10K_HTC_FLAG_BUNDLE_MASK =3D 0xF0
> };
> =20
> +#define ATH10K_HTC_FLAG_BUNDLE_LSB 4
> +
I think you could fold patches from 6 to 11 into one patch. They are all
about adding code, and most of the commit logs are empty anyway.
--=20
Kalle Valo=
^ permalink raw reply
* Re: [RFC V3 03/11] nl80211: add support for gscan
From: Johannes Berg @ 2016-12-16 10:13 UTC (permalink / raw)
To: Arend Van Spriel; +Cc: linux-wireless
In-Reply-To: <f4f83c57-3e46-b017-fa36-b3776d71670a@broadcom.com>
On Wed, 2016-12-14 at 10:01 +0100, Arend Van Spriel wrote:
> Had to look for "> 16" ;-)
Sorry.
> Here an instance of the tab vs. space issue you mentioned. Will go
> over the patch and fix that.
There were a few, not really interesting though - git would probably
flag it anyway, or checkpatch :)
> > + if (num_chans > 16)
> > + return -EINVAL;
>
> I suspect this is the restriction you were referring to.
Yes.
> There is no
> reason for this although the android wifi hal has max 16 channels in
> a bucket so I might have picked that up.
I thought I saw something with a u16 bitmap that seemed related, but I
don't see that now so I'm probably just confused.
> So could a driver have a similar limit and should we add such to the
> gscan capabilities? For instance our firmware api has a nasty
> restriction of 64 channels for all buckets together, eg. can do 4
> buckets of 16 channels each.
We do have a limit of the maximum scan buckets, which seems to be 16
right now. We also have a limit on the number of channels per bucket,
which is also 16, but no combined limit afaict (so 16x16 seems fine).
Maybe we do need some advertisement in that area then? Right now,
wifihal seems to be able to read as capabilities the number of buckets
(wifi_gscan_capabilities), but assumes the number of channels:
const unsigned MAX_CHANNELS = 16;
const unsigned MAX_BUCKETS = 16;
I guess we took that and combined it, and you had more negotiation with
Google ;-)
We may then have to actually advertise the limit you have ("64 channels
combined over all buckets"), unless you can get away with just
advertising 4 buckets (and us saying 16 channels per bucket is enough?)
I'm a bit tempted to make this more forward compatible though and not
hard-limit the number of channels per bucket in the code.
johannes
^ permalink raw reply
* Re: [PATCH v2 2/2] cfg80211: Add support to sched scan to report better BSSs
From: Johannes Berg @ 2016-12-16 9:56 UTC (permalink / raw)
To: Malinen, Jouni
Cc: Arend Van Spriel, Vamsi, Krishna, linux-wireless@vger.kernel.org
In-Reply-To: <20161215110635.GA11840@jouni.qca.qualcomm.com>
On Thu, 2016-12-15 at 11:06 +0000, Malinen, Jouni wrote:
> On Tue, Dec 13, 2016 at 04:56:55PM +0100, Johannes Berg wrote:
> > Regarding reusing attributes, we have (for the BSS selection thing)
> > the attribute NL80211_BSS_SELECT_ATTR_RSSI_ADJUST, which is really
> > quite similar to your
> > new NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF since while
> > connected (which BSS_SELECT_ATTR_* assumes) the current BSS is
> > always part of the considered BSSes, I'd think.
>
> It seems to have somewhat similar meaning, but it looks more generic
> by not being tied to just two bands (2.4 and 5). And now that I
> actually looked again at this, it does not look like
> NL80211_BSS_SELECT_ATTR_RSSI_ADJUST actually allows more than a
> single band,delta pair to be provided and as such, it actually would
> not work very well with more than two bands even if it might be a bit
> more generic by allowing band to be set to something else than 2.4 or
> 5.
Agree, it wouldn't work well with more than 2 bands. Not that we have
them now (60GHz doesn't really count here).
> By the way, nl80211.h does not seem to document what values struct
> nl82011_bss_select_rssi_adjust band uses.. Is this enum
> nl80211_band?
I believe so, we should fix that.
> Neither does it say that delta is in dB (is it?).
We should also fix that, but let's assume so for the sake of this
discussion.
> > OTOH, the new NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF doesn't
> > make this quite clear - is the current BSS to be adjusted before
> > comparing, if it's 5 GHz? If so, the semantics are equivalent. If
> > not, it doesn't actually make much sense ;-)
>
> Maybe the nl80211.h description was not clear enough, but the
> comments in cfg80211.h should be quite clear on how this was designed
> to work at the implementation level:
>
> "If the current connected BSS is in the 2.4 GHz band, other BSSs in
> the 2.4 GHz band to be reported should have better RSSI by
> @relative_rssi and other BSSs in the 5 GHz band to be reported should
> have better RSSI by (@relative_rssi - @relative_rssi_5g_pref).
> If the current connected BSS is in the 5 GHz band, other BSSs in the
> 2.4 GHz band to be reported should have better RSSI by
> (@relative_rssi + @relative_rssi_5g_pref) and other BSSs in the 5 GHz
> band to be reported should have better RSSI by by @relative_rssi."
Oh, right. Should probably be in nl80211.h too, to set expectations
from userspace.
> I'm not sure I'd describe this as adjusting the current BSS RSSI, but
> more like adjusting the RSSI threshold value if roaming would be from
> one band to another and doing that adjustment by adding or
> decrementing based on whether the roam would be from 2.4 to 5 or from
> 5 to 2.4.
It's functionally equivalent to doing the following adjustment
compare_rssi = current_bss_rssi
if current_bss_is_5ghz:
compare_rssi += relative_rssi_5g_pref
and then comparing all values (again adjusted for 5 GHz BSSes) to this.
(which is what I meant by "adjusting the current BSS RSSI").
> > So assuming that it is in fact taken into account after the same
> > adjustment, the two attributes are equivalent, and then perhaps it
> > would make sense to use struct nl80211_bss_select_rssi_adjust for
> > the new attribute. If a driver doesn't support arbitrary bands, but
> > just 5 GHz as in your example, it can just flip it around to 2.4
> > GHz by switching the sign.
> >
> > Perhaps we should even consider doing that in cfg80211 and
> > adjusting the internal API for both that way?
>
> I'm not completely sure I understood. One thing to note about
> differences here is that NL80211_BSS_SELECT_ATTR_* seems to be
> defining some preferences for BSS selection based on RSSI with an
> additional band preference, but it does not seem to define the
> threshold by how many dB the new candidate BSS should be better.
It's defining more than what we're talking about here, absolutely.
Clearly somebody thought a pure band preference might be useful (to
never connect to a 2.4GHz AP if a 5 GHz AP is available, regardless of
their relative RSSI), but that's not what we're looking at here.
(That could probably also be achieved by setting the adjustment to
90dB, but whatever)
> At minimum, we would need to clearly document struct
> nl80211_bss_select_rssi_adjust, but even if we do, I'm not sure it
> really is ideal mechanism to move to now that I realized it is not an
> array, but a single band,delta pair.
We can move to an array easily in the future by extending the attribute
length and advertising the number of array entries that are supported,
if that's your biggest concern? I don't see it as being very useful
right now since I don't think we'll see offloaded roaming between 2.4/5
and 60 GHz anytime soon. This may change when we add more bands later,
I suppose.
> Arend:
> > > I am not saying it should be avoided. Just looking at it
> > > conceptually
> > > the scheduled scan request holds so-called matchsets that specify
> > > the
> > > constraints to determine whether a BSS was found that is worth
> > > notifying the host/user-space about. As such I would expect the
> > > relative RSSI attribute(s) to be part of the matchset. That way
> > > you
> > > can specify it together with the currently connected SSID in a
> > > single
> > > matchset.
> >
> > I think this makes a lot of sense.
>
> If we are talking only about roaming within an ESS (a single SSID),
> that would sound clear, but which relative RSSI rules would apply if
> there are match sets for both the currently connected SSID and
> another SSID that the candidate BSS is for?
Right, I see how this might become a problem. I generally see no issue
with supporting multiple matchsets with different SSIDs but all having
the "relative to connected BSS RSSI filter" (which would disregard the
SSID specified in the matchset), but this then would become a problem
when multiple matchsets are specified with *different* such RSSI
filters, e.g. one matchset would specify that you want a 5G preference
of 10dB, but the other would specify a preference of only 5dB.
Conceptually in this approach, that would be supported, but firmware
likely would not be able to express this, I suppose?
> > We already have NL80211_SCHED_SCAN_MATCH_ATTR_RSSI, which asks to
> > be reporting only networks that have an *absolute* RSSI value above
> > the value of the attribute - a new attribute to make it relative to
> > the current network instead would make sense.
>
> When you say "current network", do you mean the current BSS? This
> gets complex when thinking about multiple SSIDs (which some call
> "networks") and there being NL80211_SCHED_SCAN_MATCH_ATTR_SSID and
> multiple match sets..
Yes, I did mean "current BSS". And yes - it does seem complex, see
above.
> > That would indeed be equivalent to NL80211_BSS_SELECT_ATTR_RSSI
> > then.
>
> NL80211_BSS_SELECT_ATTR_RSSI is a flag attribute.. BSS select
> mechanism does not provide an absolute RSSI value or threshold; it
> just seems to indicate use of RSSI-based selection mechanism without
> defining what exactly is better. There is
> NL80211_BSS_SELECT_ATTR_BAND_PREF that gives a preference to a
> specific band (without defining what that preference is) and then the
> NL80211_BSS_SELECT_ATTR_RSSI_ADJUST that can actually give a specific
> RSSI adjustment value (in dB?).
>
> > Now, if we consider this, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI
> > actually is equivalent to NL80211_BSS_SELECT_ATTR_RSSI (a flag
> > attribute indicating whether or not RSSI-based selection/matching
> > is done) and NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF is
> > equivalent to NL80211_BSS_SELECT_ATTR_RSSI_ADJUST, both need to be
> > given with the flag and affect operation.
>
> Hmm.. So you did notice it is a flag attribute.. So how would this
> match NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI which provides the
> threshold value for how many dB better the new BSS needs to be for it
> to be reported?
Yeah, umm, I think I got confused. There's no threshold in the
BSS_SELECT case, and maybe there doesn't need to be (at least not to
avoid host wakeups, though you'd want some hysteresis to avoid swapping
around too much).
> > So, how about we move these into NL80211_SCHED_SCAN_MATCH_ATTR_* as
> > suggested by Arend, and define them with the same content as the
> > corresponding NL80211_BSS_SELECT_ATTR_*?
>
> I think I'm missing something here.. Where would the threshold value
> (how much better new BSS needs to be) be stored? And do we really
> want something like the combination of
> NL80211_BSS_SELECT_ATTR_BAND_PREF and
> NL80211_BSS_SELECT_ATTR_RSSI_ADJUST which seems to be two different
> ways of doing band preference (the former without specifying delta
> and the latter with specific delta)? Or am I still not understanding
> how NL80211_BSS_SELECT_ATTR_* really works?
No, you're right, I missed the "better by" threshold.
I think between that (unless we add that, we could technically extend
flag attributes to allow them being an int as well, or add a new one)
and the fact that the device may not support different relative RSSI
matches in different matchsets, I'm almost convinced that adding new
attributes is better.
> The main concern I have for optional features with sched_scan is in
> whether the device ends up being woken up constantly if the driver
> does not understand a constraint that user space is trying to use to
> avoid being notified all the time.
Agree.
johannes
^ permalink raw reply
* Re: [RFC 3/3] mac80211: Add receive path for ethernet frame format
From: Johannes Berg @ 2016-12-16 9:14 UTC (permalink / raw)
To: Thiagarajan, Vasanthakumar; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1481879623.27953.4.camel@sipsolutions.net>
> > > > + return;
> > > > +
> > > > +mic_fail:
> > > > + cfg80211_michael_mic_failure(sdata->dev, sta->addr,
> > > > + (status->flag &
> > > > RX_FLAG_MCAST)
> > > > ?
> > > > + NL80211_KEYTYPE_GROUP :
> > > > + NL80211_KEYTYPE_PAIRWISE,
> > > > + key ? key->conf.keyidx :
> > > > -1,
> > > > + NULL, GFP_ATOMIC);
> > >
> > > Do we really want to handle that inline here? The driver probably
> > > has a different check to even set RX_FLAG_MMIC_ERROR, so we could
> > > just ask it to call cfg80211_michael_mic_failure() [or a wrapper
> > > to
> > > get sdata->dev] instead? I guess this works too though, and might
> > > be easier to understand.
> >
> > Yeah, driver directly reporting MIC failure will be fine. I think a
> > wrapper may be required rather than mac80211 based driver directly
> > calling cfg80211 function?
>
> It would be, because the driver can't get sdata->dev (although I
> think there's now a hidden path to do this?)
However, we can do both ways, I don't really care that much. It seems
possible though that a driver would not even report the frame, but only
the necessary info, in this case - so that we might need an out-of-band
path for it anyway?
johannes
^ permalink raw reply
* Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX
From: Johannes Berg @ 2016-12-16 9:37 UTC (permalink / raw)
To: Tamizh chelvam; +Cc: c_traja, linux-wireless, ath10k
In-Reply-To: <134cc8e58ecb804b6dda0137c4c37be8@codeaurora.org>
> > > is it fine to have as WIPHY_BTCOEX_BE_PREFERRED ?
> >
> > It's not really clear to me what you intend to do this - if it's
> > really support flags then you really should name those better.
> >
>
> This is support flags and it used by the driver to intimate driver
> supported frame type for the BTCOEX to cfg like
> "wiphy_wowlan_support_flags" implementation. Please suggest if this
> is ok ? I will be thankful if you can suggest a better one if this
> is not ok "WIPHY_BTCOEX_SUPPORTS_BE"
Well, I see a few things here:
1) does it even make sense to split it out per AC? wouldn't it be weird
if you supported this only for VO and BK, and not the others, or
something like that?
2) Wouldn't it make more sense to define this in nl80211 and just pass
the bitmap through to userspace? That would save quite a bit of netlink
mangling complexity.
3) I think the naming is confusing - "WIPHY_BTCOEX_SUPPORTS_BE_PREF" or
so might be more appropriate?
> Do you mean to say, sending a value from user space and parse that
> in the driver?
I was more thinking of the capability advertisement, but yeah, both
ways seems reasonable.
johannes
^ permalink raw reply
* Re: [RFC 1/3] mac80211: Add provision for 802.11 encap/decap offload
From: Johannes Berg @ 2016-12-16 9:30 UTC (permalink / raw)
To: Thiagarajan, Vasanthakumar; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <585273D4.7030100@qti.qualcomm.com>
On Thu, 2016-12-15 at 10:43 +0000, Thiagarajan, Vasanthakumar wrote:
> On Thursday 15 December 2016 02:46 PM, Johannes Berg wrote:
> >
> > > Drivers advertising this capability should also implement other
> > > functionalities which deal with 802.11 frame format like below
> > > - ADDBA/DELBA offload
> >
> > This shouldn't be necessary.
>
> Ok. Since driver/hw needs to implement Tx/Rx aggregation related
> functionalities I thought ADDBA/DELBA processing will be little
> important to mac80211. May be I'm missing something.
It needs to do the aggregation of the data flows, but I don't think the
control flows will need to be offloaded entirely? We'd need to have
some feedback mechanism for the SN used etc., so that might be up to
the driver to implement and might not be easy.
Anyway, I'm only suggesting to drop this from the documentation since
it doesn't seem strictly necessary, assuming the driver can report the
correct SSN in the callback all the A-MPDU setup should work more or
less as-is (except I think mac80211 won't buffer frames while doing it,
so also that would have to be done by the driver).
> > > - Powersave offload
> >
> > That's ambiguous - you do need to handle sleeping stations (and PS-
> > Poll/U-APSD) in AP mode in the device with this,
>
> I did not dig deep into PS requirement with ethernet frame format
> because frame control is not available to mac80211, so I thought to
> mention PS offload is a requirement. May be there is already an
> infrastructure in mac80211 to learn PS state of client which was
> notified in the current data frame (other than 802.11 frame control).
>
> but I don't see a deep
> > technical reason to require it for client mode. OTOH, client mode
> > is
> > almost always offloaded anyway.
>
> Ok.
Actually, come to think of it, the whole client-side powersave stuff is
so broken that I can't recommend using it - perhaps just clarify this
and say:
* AP: offloaded support for powersaving clients
* non-AP: offloaded support for powersave (if desired)
> > I think you may have forgotten one important item,
> >
> > - control port handling
>
> Hmmm, I'm getting WPA-PSK working with this. May be there are other
> control port handling which will not work with ethernet frame format?
I think I later saw control port handling in your 802.3 RX code - my
review at this point was tainted by some thoughts I had how I thought
it should work, but it's different :)
> > > + int (*get_vif_80211_hdr_offload)(struct ieee80211_hw
> > > *hw,
> > > + struct ieee80211_vif
> > > *vif,
> > > + bool is_4addr, bool
> > > *supported);
> >
> > Why are you not simply returning "supported"?
> >
> > I don't like passing the vif pointer here. At this point, the vif
> > pointer isn't known to the driver yet (through drv_add_interface)
> > so it's a dead pointer as far as the sequencing is concerned.
> >
> > Is there a possibility that drivers need to switch off ethernet
> > format handling entirely when an incompatible interface is added?
> > For example, if you add a mesh interface, is there a chance that
> > the AP interface might no longer be able to handle this?
>
> >
> > I'd hope this doesn't happen because I think that would
> be extremely
> > complicated to handle safely.
>
> Unfortunately "[RFC 2/3] mac80211: Implement data xmit for 802.11
> encap offload" tries to implement this but more likely buggy :(. You
> are right, it is very complex to get that right. May be we should not
> allow interface needing dynamic switch?
We discussed this over in the other part of the thread :)
johannes
^ permalink raw reply
* Re: [RFC 2/3] mac80211: Implement data xmit for 802.11 encap offload
From: Johannes Berg @ 2016-12-16 9:25 UTC (permalink / raw)
To: Thiagarajan, Vasanthakumar, nbd@nbd.name; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <58537DA3.1090901@qti.qualcomm.com>
On Fri, 2016-12-16 at 05:37 +0000, Thiagarajan, Vasanthakumar wrote:
> QCA988X does not have capability to configure vif specific decap
> mode. Encap mode is configurable per packet for all the ath10k based
> chips so this part should be fine to support per vif configuration.
Ok, that's good.
> Newer QCA chips like QCA9984, QCA4019, QCA9888 and QCA99X0 supports
> decap mode configuration per vif.
Also good.
> To reduce the complexity, we can probably make per vif encap/decap
> configuration mandatory to enable ethernet frame format, not sure how
> this will work with non-QCA capable hardware.
I don't know either, nobody else has talked about it yet :-)
Anyway, if we (for now) we can assume that TX can be constant 802.3
encap mode once enabled, we don't have to deal with the messy dynamic
switching you had there.
RX can switch more dynamically independent of all the mac80211 code
since it basically just means the driver calls one function or the
other - if everything is offloaded correctly there shouldn't really be
a difference.
Then also things like IEEE80211_CONF_CHANGE_80211_HDR_OFFL etc. can go
away entirely because you don't have to switch anything dynamically at
a global level. That will simplify everything a lot.
johannes
^ permalink raw reply
* Re: [RFC 3/3] mac80211: Add receive path for ethernet frame format
From: Johannes Berg @ 2016-12-16 9:13 UTC (permalink / raw)
To: Thiagarajan, Vasanthakumar; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <58538DFA.1090808@qti.qualcomm.com>
> Ok. So it is mainly for encap there needs to be some capability
> advertisement from driver
Obviously, since mac80211 needs to pick the ndo struct to use.
> because for decap driver would use appropriate mac80211 Rx function
> to indicate?
That should be sufficient, no?
In fact, for RX, assuming that the relevant things are offloaded (and
we should ensure that in the RX function(s)), the driver could be
allowed to switch around as dynamically as it wants to (as long as it's
not mixing in _irqsafe calls or doing other things that might cause
reordering)
> > > + memset(&rx, 0, sizeof(rx));
> >
> > That seems a bit pointless?
>
> Hmmm, is it not that all member other than the ones initialized will
> be junk and might result in crash when accessing uninitialized
> pointer member like napi?
Oh, ok. I thought it was actually all filled.
You really should support NAPI though.
> > Yeah, that's not nice - perhaps we can provide the TID out of band?
>
> Right. This is possible with ath10k where 802.11 header is also
> available in Rx desc of the frame irrespective of the format of the
> Rx payload
We could go with that for now then. Leave a comment indicating that if
it can't be provided, we need to disable statistics for it?
> >
> > > + return;
> > > +
> > > +mic_fail:
> > > + cfg80211_michael_mic_failure(sdata->dev, sta->addr,
> > > + (status->flag &
> > > RX_FLAG_MCAST)
> > > ?
> > > + NL80211_KEYTYPE_GROUP :
> > > + NL80211_KEYTYPE_PAIRWISE,
> > > + key ? key->conf.keyidx :
> > > -1,
> > > + NULL, GFP_ATOMIC);
> >
> > Do we really want to handle that inline here? The driver probably
> > has a different check to even set RX_FLAG_MMIC_ERROR, so we could
> > just ask it to call cfg80211_michael_mic_failure() [or a wrapper to
> > get sdata->dev] instead? I guess this works too though, and might
> > be easier to understand.
>
> Yeah, driver directly reporting MIC failure will be fine. I think a
> wrapper may be required rather than mac80211 based driver directly
> calling cfg80211 function?
It would be, because the driver can't get sdata->dev (although I think
there's now a hidden path to do this?)
johannes
^ permalink raw reply
* Re: [PATCH mac80211-next] rfkill: hide unused goto label
From: Johannes Berg @ 2016-12-16 9:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, João Paulo Rechi Vita,
Michał Kępień, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20161216084425.1692828-1-arnd@arndb.de>
On Fri, 2016-12-16 at 09:44 +0100, Arnd Bergmann wrote:
> A cleanup introduced a harmless warning in some configurations:
>
> net/rfkill/core.c: In function 'rfkill_init':
> net/rfkill/core.c:1350:1: warning: label 'error_input' defined but
> not used [-Wunused-label]
>
> This adds another #ifdef around the label to match that around the
> caller.
Applied, thanks.
johannes
^ permalink raw reply
* [PATCH mac80211-next] rfkill: hide unused goto label
From: Arnd Bergmann @ 2016-12-16 8:44 UTC (permalink / raw)
To: Johannes Berg
Cc: Arnd Bergmann, David S. Miller, João Paulo Rechi Vita,
Michał Kępień, linux-wireless, netdev,
linux-kernel
A cleanup introduced a harmless warning in some configurations:
net/rfkill/core.c: In function 'rfkill_init':
net/rfkill/core.c:1350:1: warning: label 'error_input' defined but not used [-Wunused-label]
This adds another #ifdef around the label to match that around the
caller.
Fixes: 6124c53edeea ("rfkill: Cleanup error handling in rfkill_init()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/rfkill/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index f6b01f3616e5..b7adaee69756 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -1347,8 +1347,10 @@ static int __init rfkill_init(void)
return 0;
+#ifdef CONFIG_RFKILL_INPUT
error_input:
rfkill_any_led_trigger_unregister();
+#endif
error_led_trigger:
misc_deregister(&rfkill_miscdev);
error_misc:
--
2.9.0
^ permalink raw reply related
* [PATCH 1/2] mwifiex: change width of MAC control variable
From: Amitkumar Karwar @ 2016-12-16 7:25 UTC (permalink / raw)
To: linux-wireless; +Cc: Cathy Luo, Nishant Sarmukadam, Amitkumar Karwar
Firmware has started making use of reserved field.
Accordingly change curr_pkt_filter from u16 to u32.
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
drivers/net/wireless/marvell/mwifiex/fw.h | 18 ++++++++----------
drivers/net/wireless/marvell/mwifiex/main.h | 2 +-
drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 4 ++--
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index ea45594..f48247c 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -434,14 +434,13 @@ enum mwifiex_channel_flags {
#define HostCmd_ACT_BITWISE_SET 0x0002
#define HostCmd_ACT_BITWISE_CLR 0x0003
#define HostCmd_RESULT_OK 0x0000
-
-#define HostCmd_ACT_MAC_RX_ON 0x0001
-#define HostCmd_ACT_MAC_TX_ON 0x0002
-#define HostCmd_ACT_MAC_WEP_ENABLE 0x0008
-#define HostCmd_ACT_MAC_ETHERNETII_ENABLE 0x0010
-#define HostCmd_ACT_MAC_PROMISCUOUS_ENABLE 0x0080
-#define HostCmd_ACT_MAC_ALL_MULTICAST_ENABLE 0x0100
-#define HostCmd_ACT_MAC_ADHOC_G_PROTECTION_ON 0x2000
+#define HostCmd_ACT_MAC_RX_ON BIT(0)
+#define HostCmd_ACT_MAC_TX_ON BIT(1)
+#define HostCmd_ACT_MAC_WEP_ENABLE BIT(3)
+#define HostCmd_ACT_MAC_ETHERNETII_ENABLE BIT(4)
+#define HostCmd_ACT_MAC_PROMISCUOUS_ENABLE BIT(7)
+#define HostCmd_ACT_MAC_ALL_MULTICAST_ENABLE BIT(8)
+#define HostCmd_ACT_MAC_ADHOC_G_PROTECTION_ON BIT(13)
#define HostCmd_BSS_MODE_IBSS 0x0002
#define HostCmd_BSS_MODE_ANY 0x0003
@@ -1084,8 +1083,7 @@ struct host_cmd_ds_802_11_mac_address {
};
struct host_cmd_ds_mac_control {
- __le16 action;
- __le16 reserved;
+ __le32 action;
};
struct host_cmd_ds_mac_multicast_adr {
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 5f7a010..5c82972 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -529,7 +529,7 @@ struct mwifiex_private {
u8 tx_timeout_cnt;
struct net_device *netdev;
struct net_device_stats stats;
- u16 curr_pkt_filter;
+ u32 curr_pkt_filter;
u32 bss_mode;
u32 pkt_tx_ctrl;
u16 tx_power_level;
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
index ad2dfd8..2f1f4d1 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
@@ -76,7 +76,7 @@
*/
static int mwifiex_cmd_mac_control(struct mwifiex_private *priv,
struct host_cmd_ds_command *cmd,
- u16 cmd_action, u16 *action)
+ u16 cmd_action, u32 *action)
{
struct host_cmd_ds_mac_control *mac_ctrl = &cmd->params.mac_ctrl;
@@ -89,7 +89,7 @@ static int mwifiex_cmd_mac_control(struct mwifiex_private *priv,
cmd->command = cpu_to_le16(HostCmd_CMD_MAC_CONTROL);
cmd->size =
cpu_to_le16(sizeof(struct host_cmd_ds_mac_control) + S_DS_GEN);
- mac_ctrl->action = cpu_to_le16(*action);
+ mac_ctrl->action = cpu_to_le32(*action);
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] mwifiex: Enable dynamic bandwidth signalling
From: Amitkumar Karwar @ 2016-12-16 7:25 UTC (permalink / raw)
To: linux-wireless; +Cc: Cathy Luo, Nishant Sarmukadam, Amitkumar Karwar
In-Reply-To: <1481873145-5381-1-git-send-email-akarwar@marvell.com>
Enable dynamic bandwidth signalling by setting the corresponding
bit in MAC control register.
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
drivers/net/wireless/marvell/mwifiex/fw.h | 1 +
drivers/net/wireless/marvell/mwifiex/init.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index f48247c..55db158f 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -441,6 +441,7 @@ enum mwifiex_channel_flags {
#define HostCmd_ACT_MAC_PROMISCUOUS_ENABLE BIT(7)
#define HostCmd_ACT_MAC_ALL_MULTICAST_ENABLE BIT(8)
#define HostCmd_ACT_MAC_ADHOC_G_PROTECTION_ON BIT(13)
+#define HostCmd_ACT_MAC_DYNAMIC_BW_ENABLE BIT(16)
#define HostCmd_BSS_MODE_IBSS 0x0002
#define HostCmd_BSS_MODE_ANY 0x0003
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index 87cda4f..7569483 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -92,7 +92,8 @@ int mwifiex_init_priv(struct mwifiex_private *priv)
for (i = 0; i < ARRAY_SIZE(priv->wep_key); i++)
memset(&priv->wep_key[i], 0, sizeof(struct mwifiex_wep_key));
priv->wep_key_curr_index = 0;
- priv->curr_pkt_filter = HostCmd_ACT_MAC_RX_ON | HostCmd_ACT_MAC_TX_ON |
+ priv->curr_pkt_filter = HostCmd_ACT_MAC_DYNAMIC_BW_ENABLE |
+ HostCmd_ACT_MAC_RX_ON | HostCmd_ACT_MAC_TX_ON |
HostCmd_ACT_MAC_ETHERNETII_ENABLE;
priv->beacon_period = 100; /* beacon interval */
--
1.9.1
^ permalink raw reply related
* Re: wl1251 & mac address & calibration data
From: Daniel Wagner @ 2016-12-16 7:25 UTC (permalink / raw)
To: Luis R. Rodriguez, Arend Van Spriel, Tom Gundersen, Johannes Berg,
Ming Lei, Mimi Zohar, Bjorn Andersson, Rafał Miłecki
Cc: Pali Rohár, Kalle Valo, Sebastian Reichel, Pavel Machek,
Michal Kazior, Ivaylo Dimitrov, Aaro Koskinen, Tony Lindgren,
linux-wireless, Network Development, linux-kernel@vger.kernel.org,
David Woodhouse, Takashi Iwai, Josh Boyer, Dmitry Torokhov
In-Reply-To: <CAB=NE6VHF+U7DzapENvLMKtN4Dgi9J0qMoeUJ8o4vSwc2-rRFQ@mail.gmail.com>
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 don't see any reason why firmwared
should not also support loading calibration data. If we find a sound way
to do this.
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
^ permalink raw reply
* Re: [RFC 3/3] mac80211: Add receive path for ethernet frame format
From: Thiagarajan, Vasanthakumar @ 2016-12-16 6:47 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1481794695.31776.7.camel@sipsolutions.net>
T24gVGh1cnNkYXkgMTUgRGVjZW1iZXIgMjAxNiAwMzowOCBQTSwgSm9oYW5uZXMgQmVyZyB3cm90
ZToNCj4NCj4+IFRoaXMgcnggcGF0aCBvbmx5IGNoZWNrcyBpZiB0aGUgZHJpdmVyIGhhcyBhZHZl
cnRpc2VkDQo+PiBpdCdzIHN1cHBvcnQgb2YgODAyLjExIGhlYWRlciBlbmNhcC9kZWNhcCBvZmZs
b2FkIGZvcg0KPj4gZGF0YSBmcmFtZXMuDQo+DQo+IEknbSBub3QgZXZlbiBzdXJlIEkgc2VlIHRo
ZSBwb2ludCBpbiB0aGF0PyBPdGhlciB0aGFuIHRoYXQgKGFuZCB0aGUNCj4gdmFyaW91cyBvdGhl
ciBvZmZsb2FkIHJlcXVpcmVtZW50cyksIGl0IHNlZW1zIHRoYXQgZW5jYXAvZGVjYXAgY291bGQg
YmUNCj4gY29uc2lkZXJlZCBvcnRob2dvbmFsLg0KDQpPay4gU28gaXQgaXMgbWFpbmx5IGZvciBl
bmNhcCB0aGVyZSBuZWVkcyB0byBiZSBzb21lIGNhcGFiaWxpdHkgYWR2ZXJ0aXNlbWVudA0KZnJv
bSBkcml2ZXIgYmVjYXVzZSBmb3IgZGVjYXAgZHJpdmVyIHdvdWxkIHVzZSBhcHByb3ByaWF0ZSBt
YWM4MDIxMSBSeCBmdW5jdGlvbg0KdG8gaW5kaWNhdGU/DQoNCj4NCj4+ICsJICogQWRob2MgaW50
ZXJmYWNlIGRlcGVuZHMgb24gYnNzaWQgdG8gdWRwYXRlIGxhc3RfcnguDQo+DQo+IHR5cGUgLSB1
cGRhdGUNCj4NCj4+ICsJaWYgKCEoc3RhdHVzLT5mbGFnICYgUlhfRkxBR19NQ0FTVCkpIHsNCj4+
ICsJCXN0YS0+cnhfc3RhdHMubGFzdF9yeCA9IGppZmZpZXM7DQo+PiArCQlzdGEtPnJ4X3N0YXRz
Lmxhc3RfcmF0ZSA9DQo+PiBzdGFfc3RhdHNfZW5jb2RlX3JhdGUoc3RhdHVzKTsNCj4+ICsJfQ0K
Pg0KPiBZb3Ugc2hvdWxkIHByb2JhYmx5IHJlbmFtZSB0aGF0IGZsYWcgdG8gc29tZXRoaW5nIGxp
a2UNCj4gUlhfRkxBR184MDIxMV9NQ0FTVCBzaW5jZSBvdGhlcndpc2UgaXQncyBjb25mdXNpbmcg
d2l0aCB0aGUgbmV4dA0KPiBtdWx0aWNhc3QgZXRoZXIgYWRkciBjaGVjazoNCg0KU3VyZS4NCg0K
Pg0KPj4gKwlpZiAoc2RhdGEtPnZpZi50eXBlID09IE5MODAyMTFfSUZUWVBFX1NUQVRJT04gJiYN
Cj4+ICsJICAgICFpc19tdWx0aWNhc3RfZXRoZXJfYWRkcihlaGRyLT5oX2Rlc3QpKQ0KPj4gKwkJ
aWVlZTgwMjExX3N0YV9yZXNldF9jb25uX21vbml0b3Ioc2RhdGEpOw0KPg0KPiBCdXQgdGhpcyBj
b3VsZCBqdXN0IGFsc28gdXNlIHRoZSBmbGFnLCBzaW5jZSBpbiBzdGF0aW9uIG1vZGUgdGhlIHR3
bw0KPiBhcmUgZXF1aXZhbGVudCwgYW5kIGl0J2QgYmUgZWFzaWVyIHRvIGZpZ3VyZSBvdXQgaWYg
dGhpcyB3YXMgImVsc2UgaWYNCj4gKHN0YXRpb24gbW9kZSkiPw0KDQpBaCwgY29ycmVjdC4NCg0K
Pg0KPj4gKwltZW1zZXQoJnJ4LCAwLCBzaXplb2YocngpKTsNCj4NCj4gVGhhdCBzZWVtcyBhIGJp
dCBwb2ludGxlc3M/DQoNCkhtbW0sIGlzIGl0IG5vdCB0aGF0IGFsbCBtZW1iZXIgb3RoZXIgdGhh
biB0aGUgb25lcyBpbml0aWFsaXplZCB3aWxsIGJlIGp1bmsgYW5kIG1pZ2h0DQpyZXN1bHQgaW4g
Y3Jhc2ggd2hlbiBhY2Nlc3NpbmcgdW5pbml0aWFsaXplZCBwb2ludGVyIG1lbWJlciBsaWtlIG5h
cGk/DQoNCj4NCj4+ICsJcnguc2tiID0gc2tiOw0KPj4gKwlyeC5zZGF0YSA9IHNkYXRhOw0KPj4g
KwlyeC5sb2NhbCA9IGxvY2FsOw0KPj4gKwlyeC5zdGEgPSBzdGE7DQo+PiArDQo+PiArCWlmICh2
aWYtPnR5cGUgPT0gTkw4MDIxMV9JRlRZUEVfQVBfVkxBTiAmJiBzZGF0YS0+YnNzICYmDQo+PiAr
CSAgICB1bmxpa2VseShlaGRyLT5oX3Byb3RvID09IHNkYXRhLT5jb250cm9sX3BvcnRfcHJvdG9j
b2wpKQ0KPj4gew0KPj4gKwkJc2RhdGEgPSBjb250YWluZXJfb2Yoc2RhdGEtPmJzcywgc3RydWN0
DQo+PiBpZWVlODAyMTFfc3ViX2lmX2RhdGEsDQo+PiArCQkJCSAgICAgdS5hcCk7DQo+PiArCQlk
ZXYgPSBzZGF0YS0+ZGV2Ow0KPj4gKwkJcnguc2RhdGEgPSBzZGF0YTsNCj4+ICsJfQ0KPj4gKw0K
Pj4gKwlyeC5za2ItPmRldiA9IGRldjsNCj4+ICsNCj4+ICsJLyogWFhYOiBTaW5jZSByeC5zZXFu
b19pZHggaXMgbm90IGF2YWlsYWJsZSBmb3IgZGVjYXANCj4+IG9mZmxvYWRlZCBmcmFtZXMNCj4+
ICsJICogcnggbXNkdSBzdGF0cyB1cGRhdGUgYXQgdGhlIHNlcW5vX2lkeCBpbg0KPj4gaWVlZTgw
MjExX2RlbGl2ZXJfc2tiKCkNCj4+ICsJICogd2lsbCBhbHdheXMgYmUgdXBkYXRlZCBhdCBpbmRl
eCAwIGFuZCB3aWxsIG5vdCBiZSB2ZXJ5DQo+PiB1c2VmdWwuDQo+PiArCSAqLw0KPj4gKwlpZWVl
ODAyMTFfZGVsaXZlcl9za2IoJnJ4KTsNCj4NCj4gWWVhaCwgdGhhdCdzIG5vdCBuaWNlIC0gcGVy
aGFwcyB3ZSBjYW4gcHJvdmlkZSB0aGUgVElEIG91dCBvZiBiYW5kPw0KDQpSaWdodC4gVGhpcyBp
cyBwb3NzaWJsZSB3aXRoIGF0aDEwayB3aGVyZSA4MDIuMTEgaGVhZGVyIGlzIGFsc28gYXZhaWxh
YmxlIGluIFJ4DQpkZXNjIG9mIHRoZSBmcmFtZSBpcnJlc3BlY3RpdmUgb2YgdGhlIGZvcm1hdCBv
ZiB0aGUgUnggcGF5bG9hZA0KDQogIElmDQo+IG5vdCwgd2UnbGwgaGF2ZSB0byBkaXNhYmxlIHRo
b3NlIHN0YXRpc3RpY3MgKmFsbCB0aGUgd2F5KiwgaS5lLiBub3QNCj4gZXZlbiByZXBvcnQgdGhl
bSB0byB1c2Vyc3BhY2Ugd2hlbiBmaWxsaW5nIHNpbmZvLg0KDQpZZXMsIHRoaXMgbWF5IGJlIHRo
ZSBvcHRpb24gd2hlbiBkcml2ZXIgaXMgdW5hd2FyZSBvZiByeCBwYWNrZXQgdGlkLg0KDQo+DQo+
PiArCXJldHVybjsNCj4+ICsNCj4+ICttaWNfZmFpbDoNCj4+ICsJY2ZnODAyMTFfbWljaGFlbF9t
aWNfZmFpbHVyZShzZGF0YS0+ZGV2LCBzdGEtPmFkZHIsDQo+PiArCQkJCSAgICAgKHN0YXR1cy0+
ZmxhZyAmIFJYX0ZMQUdfTUNBU1QpDQo+PiA/DQo+PiArCQkJCSAgICAgTkw4MDIxMV9LRVlUWVBF
X0dST1VQIDoNCj4+ICsJCQkJICAgICBOTDgwMjExX0tFWVRZUEVfUEFJUldJU0UsDQo+PiArCQkJ
CSAgICAga2V5ID8ga2V5LT5jb25mLmtleWlkeCA6IC0xLA0KPj4gKwkJCQkgICAgIE5VTEwsIEdG
UF9BVE9NSUMpOw0KPg0KPiBEbyB3ZSByZWFsbHkgd2FudCB0byBoYW5kbGUgdGhhdCBpbmxpbmUg
aGVyZT8gVGhlIGRyaXZlciBwcm9iYWJseSBoYXMgYQ0KPiBkaWZmZXJlbnQgY2hlY2sgdG8gZXZl
biBzZXQgUlhfRkxBR19NTUlDX0VSUk9SLCBzbyB3ZSBjb3VsZCBqdXN0IGFzayBpdA0KPiB0byBj
YWxsIGNmZzgwMjExX21pY2hhZWxfbWljX2ZhaWx1cmUoKSBbb3IgYSB3cmFwcGVyIHRvIGdldCBz
ZGF0YS0+ZGV2XQ0KPiBpbnN0ZWFkPyBJIGd1ZXNzIHRoaXMgd29ya3MgdG9vIHRob3VnaCwgYW5k
IG1pZ2h0IGJlIGVhc2llciB0bw0KPiB1bmRlcnN0YW5kLg0KDQpZZWFoLCBkcml2ZXIgZGlyZWN0
bHkgcmVwb3J0aW5nIE1JQyBmYWlsdXJlIHdpbGwgYmUgZmluZS4gSSB0aGluayBhIHdyYXBwZXIN
Cm1heSBiZSByZXF1aXJlZCByYXRoZXIgdGhhbiBtYWM4MDIxMSBiYXNlZCBkcml2ZXIgZGlyZWN0
bHkgY2FsbGluZyBjZmc4MDIxMQ0KZnVuY3Rpb24/DQoNClZhc2FudGg=
^ permalink raw reply
* Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX
From: Tamizh chelvam @ 2016-12-16 5:53 UTC (permalink / raw)
To: Johannes Berg; +Cc: c_traja, linux-wireless, ath10k
In-Reply-To: <1481645351.20412.34.camel@sipsolutions.net>
Hi Johannes,
Thanks for the comments
On 2016-12-13 21:39, Johannes Berg wrote:
>> > > /**
>> > > + * wiphy_btcoex_support_flags
>> > > + * This enum has the driver supported frame types for
>> > > BTCOEX.
>> > > + * @WIPHY_WLAN_BE_PREFERRED - Supports Best Effort frame for
>> > > BTCOEX
>> > > + * @WIPHY_WLAN_BK_PREFERRED - supports Background frame for
>> > > BTCOEX
>> > > + * @WIPHY_WLAN_VI_PREFERRED - supports Video frame for BTCOEX
>> > > + * @WIPHY_WLAN_VO_PREFERRED - supports Voice frame for BTCOEX
>> > > + * @WIPHY_WLAN_BEACON_PREFERRED - supports Beacon frame for
>> > > BTCOEX
>> > > + * @WIPHY_WLAN_MGMT_PREFERRED - supports Management frames for
>> > > BTCOEX.
>> > > + */
>> >
>> > That's not making much sense to me?
>> >
>>
>> is it fine to have as WIPHY_BTCOEX_BE_PREFERRED ?
>
> It's not really clear to me what you intend to do this - if it's really
> support flags then you really should name those better.
>
This is support flags and it used by the driver to intimate driver
supported frame type
for the BTCOEX to cfg like "wiphy_wowlan_support_flags" implementation.
Please suggest if this is ok ? I will be thankful if you can suggest a
better one if this is not ok
"WIPHY_BTCOEX_SUPPORTS_BE"
>> > > +/**
>> > > + * enum wiphy_btcoex_priority - BTCOEX priority level
>> > > + * This enum defines priority level for BTCOEX
>> > > + * WIPHY_WLAN_PREFERRED_LOW - low priority frames over BT
>> > > traffic
>> > > + * WIPHY_WLAN_PREFERRED_HIGH - high priority frames over BT
>> > > traffic
>> > > + */
>> > > +
>> > > +enum wiphy_btcoex_priority {
>> > > + WIPHY_WLAN_PREFERRED_LOW = false,
>> > > + WIPHY_WLAN_PREFERRED_HIGH = true,
>> > > +};
>> >
>> > That false/true seems just strange.
>> >
>>
>> I will just use as a enum without assigning false/true.
>
> What do you even need this enum for though?
>
Ok. I will directly assign true for the flag.
>> > > +enum nl80211_btcoex_priority {
>> > > + __NL80211_WLAN_PREFERRED_INVALID,
>> > > + NL80211_WLAN_BE_PREFERRED,
>> > > + NL80211_WLAN_BK_PREFERRED,
>> > > + NL80211_WLAN_VI_PREFERRED,
>> > > + NL80211_WLAN_VO_PREFERRED,
>> > > + NL80211_WLAN_BEACON_PREFERRED,
>> > > + NL80211_WLAN_MGMT_PREFERRED,
>> > > + __NL80211_WLAN_PREFERRED_LAST,
>> > > + NL80211_WLAN_PREFERRED_MAX =
>> > > + __NL80211_WLAN_PREFERRED_LAST - 1,
>> > > +};
>> >
>> > Wouldn't a bitmap be easier?
>> >
>> since this is to distinguish between different btcoex priorities and
>> we
>> are not going to do any manipulations on these parameters.
>> It is just used as flag attribute.
>
> But why the (parsing) complexity, when a single bitmap would do?
>
Do you mean to say, sending a value from user space and parse that in
the driver?
^ permalink raw reply
* Re: [RFC 2/3] mac80211: Implement data xmit for 802.11 encap offload
From: Thiagarajan, Vasanthakumar @ 2016-12-16 5:37 UTC (permalink / raw)
To: Johannes Berg, nbd@nbd.name; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1481809984.2582.5.camel@sipsolutions.net>
T24gVGh1cnNkYXkgMTUgRGVjZW1iZXIgMjAxNiAwNzoyMyBQTSwgSm9oYW5uZXMgQmVyZyB3cm90
ZToNCj4NCj4+PiBJIGFncmVlLiBEeW5hbWljIHN3aXRjaCBwYXJ0IGlzIGJ1Z2d5LCB3ZSBjYW4g
c3RhcnQgd2l0aCBub3QNCj4+PiBhbGxvd2luZyBpbnRlcmZhY2VzIHJlc3VsdGluZyBpbiBkeW5h
bWljIHN3aXRjaC4NCj4+DQo+PiBEb2VzIHRoaXMgbWVhbiB0aGF0IHdoZW4gYnJpbmdpbmcgdXAg
bXVsdGlwbGUgaW50ZXJmYWNlcywgdXNlcnMgd291bGQNCj4+IG5lZWQgdG8gZmlndXJlIG91dCB0
aGUgJ21hZ2ljJyBvcmRlciB0aGF0IHdvcmtzPw0KPg0KPiBJIHRoaW5rIHdlIG5lZWQgdG8gdGFs
ayBhYm91dCBoYXJkd2FyZSBjYXBhYmlsaXRpZXMgYXQgdGhpcyBwb2ludC4NCg0KUUNBOTg4WCBk
b2VzIG5vdCBoYXZlIGNhcGFiaWxpdHkgdG8gY29uZmlndXJlIHZpZiBzcGVjaWZpYyBkZWNhcCBt
b2RlLiBFbmNhcCBtb2RlDQppcyBjb25maWd1cmFibGUgcGVyIHBhY2tldCBmb3IgYWxsIHRoZSBh
dGgxMGsgYmFzZWQgY2hpcHMgc28gdGhpcyBwYXJ0IHNob3VsZCBiZQ0KZmluZSB0byBzdXBwb3J0
IHBlciB2aWYgY29uZmlndXJhdGlvbi4gTmV3ZXIgUUNBIGNoaXBzIGxpa2UgUUNBOTk4NCwgUUNB
NDAxOSwgUUNBOTg4OA0KYW5kIFFDQTk5WDAgc3VwcG9ydHMgZGVjYXAgbW9kZSBjb25maWd1cmF0
aW9uIHBlciB2aWYuIFRvIHJlZHVjZSB0aGUgY29tcGxleGl0eSwNCndlIGNhbiBwcm9iYWJseSBt
YWtlIHBlciB2aWYgZW5jYXAvZGVjYXAgY29uZmlndXJhdGlvbiBtYW5kYXRvcnkgdG8gZW5hYmxl
IGV0aGVybmV0DQpmcmFtZSBmb3JtYXQsIG5vdCBzdXJlIGhvdyB0aGlzIHdpbGwgd29yayB3aXRo
IG5vbi1RQ0EgY2FwYWJsZSBoYXJkd2FyZS4NCg0KPg0KPiBJIHdhcyBhc3N1bWluZyB0aGF0IGl0
IHdvdWxkIGFjdHVhbGx5IGJlIHBvc3NpYmxlIHRvIHJ1biB0d28gaW50ZXJmYWNlcw0KPiB3aXRo
IGRpZmZlcmVudCBwYXRocyBoZXJlIGNvbmN1cnJlbnRseSAtIGlzIHRoYXQgbm90IHRydWU/IElm
IHRoYXQncw0KPiBub3QgdHJ1ZSwgdGhlbiB3ZSBhYnNvbHV0ZWx5IF9uZWVkXyBkeW5hbWljIHN3
aXRjaGluZywgSSBhZ3JlZSB3aXRoDQo+IEZlbGl4LCBidXQgdGhlbiB3ZSBoYXZlIGEgcHJldHR5
IGJpZyBjb21wbGljYXRpb24gdG8gZmlndXJlIG91dC4gQnV0IHdlDQo+IGNhbid0IGxldCB0aGlz
IG9wdGltaXNhdGlvbiBhZmZlY3QgdXNlciBleHBlcmllbmNlLg0KDQpTdXJlLg0KDQpWYXNhbnRo
^ permalink raw reply
* Re: [PATCH 1/2] ath10k: add accounting for the extended peer statistics
From: Mohammed Shafi Shajakhan @ 2016-12-16 5:24 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, ath10k, Kalle Valo
In-Reply-To: <2970240.ih558OrmaK@debian64>
Hi Christian,
> On Thursday, December 15, 2016 10:13:39 PM CET Mohammed Shafi Shajakhan wrote:
> > I am also thinking, as of now there is not much use in appending
> > the extended peer stats (which gets periodically ) to the linked list
> > '&ar->debug.fw_stats.peers_extd)' and should we get rid of the below
> > (and the required cleanup as well)
> >
> > list_splice_tail_init(&stats.peers_extd,
> > &ar->debug.fw_stats.peers_extd);
> >
> >
> > since rx_duration is getting updated periodically to the per sta
> > information. Kindly let me know your thoughts about this.
>
> Yes, of course. I see what your are trying to do and I think it's much better
> to get rid of peers_extd and ath10k_fw_extd_stats_peers_free.
>
[shafi] Feel free to post the change and I can test the same for you(next week) !
Let me know if you are busy on something else, I can take this up.
As discussed, the fix to free 'extd stats' when number of peers exceeds the range
is definitely needed. Thank you for looking into this.
thanks,
shafi
^ permalink raw reply
* Re: [Patch] NFC: trf7970a:
From: Mark Greer @ 2016-12-16 4:52 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
Samuel Ortiz, Justin Bronder
In-Reply-To: <20161214223123.GA10281@animalcreek.com>
On Wed, Dec 14, 2016 at 03:31:23PM -0700, Mark Greer wrote:
> I'll start on this
> tonight but won't likely get far until tomorrow. In the meantime,
> if you and/or your contractor make progress, please share.
Geoff,
Which version of neard are you using? 0.16?
Mark
--
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Luis R. Rodriguez @ 2016-12-16 2:03 UTC (permalink / raw)
To: Arend Van Spriel, Tom Gundersen, Daniel Wagner, Johannes Berg,
Ming Lei, Mimi Zohar, Bjorn Andersson, Rafał Miłecki
Cc: Pali Rohár, Kalle Valo, Sebastian Reichel, Pavel Machek,
Michal Kazior, Ivaylo Dimitrov, Aaro Koskinen, Tony Lindgren,
linux-wireless, Network Development, linux-kernel@vger.kernel.org,
David Woodhouse, Takashi Iwai, Josh Boyer, Dmitry Torokhov
In-Reply-To: <76365770-f5ba-9565-3fca-710518f64d81@broadcom.com>
On Thu, Dec 15, 2016 at 2:12 PM, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 15-12-2016 16:33, Pali Rohár wrote:
>> On Thu Dec 15 09:18:44 2016 Kalle Valo <kvalo@codeaurora.org> wrote:
>>> (Adding Luis because he has been working on request_firmware() lately)
>>>
>>> Pali Rohár <pali.rohar@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" persi in this
API, that was one issue with the design of the API, but there are much
more. The entire change in mechanism of "firmware fallback mechanism"
depending on which kernel config options you have is another. Its a
big web of gum put together. All this needs to end.
I've recently proposed a new patch to first help with understanding
each of the individual items I've listed above with as much new
information as is possible. I myself need to re-read it to just keep
tabs on what the hell is going on at times. After this I have a new
API proposal which I've been working on for a long time now which adds
an extensible API with only 2 calls: sync, async, and lets us modify
attributes through a parameters struct. This is what we should use in
the future for further extensions.
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.
I'll note -- the "custom fallback mechanism", part of the old API is
just a terrible idea for many reasons. I've documented issues in my
documentation revamp, I suggest to read that, refer to this thread:
https://lkml.kernel.org/r/20161213030828.17820-4-mcgrof@kernel.org
> The idea was to have a
> dedicated API call that explicitly does the request towards user-space.
In so far as this driver example that was mentioned in this thread my
recommendation is to use the default existing MAC address /
calibration data on /lib/firmware/ and then use another request to
help override for the custom thing. The only issue of course is that
the timeout for the custom thing is 60 seconds, but if your platforms
are controlled -- just reduce this to 1 second or so given that udev
is long gone and it would seem you'd only use a custom fw request to
depend on. You could also wrap this thing into a kconfig option for
now, and have the filename specified, if empty then no custom request
is sent. If set then you have it request it.
Please note the other patches in my series for the custom fallback
though. We have only 2 users upstream -- and from recent discussions
it would seem it might be possible to address what you need with
firmwared in a clean way without this custom old fallback crap thing.
Johannes at least has a similar requirement and is convinced a
"custom" thing can be done without even adding an extra custom kobject
uevent attribute as from what I recall he hinted, drivers have no
business to be involved in this. If you do end up using the custom
fallback mechanism be sure to add super crystal clear documentation
for it (see my other patches for the declarer for this documentation).
Since we have only 2 drivers using this custom thing its hard to get
folks to commit to writing the docs, write it now and be sure you
think of the future as well.
Oh also the "custom firmware fallback" was also broken on recent
kernels for many kernel revisions, it just did not work until recently
a fix which went in, so your users wills need this fix cherry picked.
Hey I tell you, the custom fw thing was a terrible incarnation. Only 2
drivers use it. There are good reasons to not like it (be sure to read
the suspend issue I documented).
> By the way, are we talking here about wl1251 device or driver as you
> also mentioned wl12xx? I did not read the entire thread.
Luis
^ permalink raw reply
* Re: [PATCH 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Mark Greer @ 2016-12-16 1:18 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: <1481841044-4314-3-git-send-email-glansberry@gmail.com>
On Thu, Dec 15, 2016 at 05:30:44PM -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 curent state of the trf7970a is reading but
> a request has been made to send a command.
>
> The solution is to properly kill the waiting reading (workqueue)
> before failing on the send.
Maybe its just me but I find this description a little hard to grok.
Mind reworking it?
The patch itself looks fine.
Thanks,
Mark
--
^ permalink raw reply
* Re: [PATCH 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
From: Mark Greer @ 2016-12-16 1:13 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo, robh+dt,
mark.rutland, netdev, devicetree, linux-kernel, justin
In-Reply-To: <1481841044-4314-2-git-send-email-glansberry@gmail.com>
On Thu, Dec 15, 2016 at 05:30:43PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <geoff@kuvee.com>
Missing commit description.
> diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
> index 2d2a077..b4c37ab 100644
> --- a/drivers/nfc/trf7970a.c
> +++ b/drivers/nfc/trf7970a.c
> @@ -1048,6 +1049,11 @@ static int trf7970a_init(struct trf7970a *trf)
> if (ret)
> goto err_out;
>
> + ret = trf7970a_write(trf, TRF7970A_REG_IO_CTRL,
> + trf->io_ctrl|TRF7970A_REG_IO_CTRL_VRS(0x1));
s/l|T/l | T/
Otherwise, looks good.
Mark
--
^ permalink raw reply
* Re: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock
From: Mark Greer @ 2016-12-16 1:06 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo, robh+dt,
mark.rutland, netdev, devicetree, linux-kernel, justin
In-Reply-To: <1481841044-4314-1-git-send-email-glansberry@gmail.com>
Hi Geoff.
On Thu, Dec 15, 2016 at 05:30:42PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <geoff@kuvee.com>
Please add an informative commit description to all of your commits.
No matter how trivial this patch may seem to you now, it may not be
to others (or to you in a few years).
> diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
> index 26c9dbb..2d2a077 100644
> --- a/drivers/nfc/trf7970a.c
> +++ b/drivers/nfc/trf7970a.c
> @@ -1181,27 +1180,37 @@ static int trf7970a_in_config_rf_tech(struct trf7970a *trf, int tech)
> switch (tech) {
> case NFC_DIGITAL_RF_TECH_106A:
> trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_14443A_106;
> - trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
> + trf->modulator_sys_clk_ctrl =
> + (trf->modulator_sys_clk_ctrl & 0xF8) |
nit: s/0xF8/0xf8/ please (for consistency with the rest of the file.).
Otherwise, it looks good.
Thanks,
Mark
^ permalink raw reply
* Re: ath10k firmware sends probes on DFS channels without radar detection
From: Jouni Malinen @ 2016-12-15 22:58 UTC (permalink / raw)
To: Jean-Pierre Tosoni; +Cc: 'Ben Greear', linux-wireless, ath10k
In-Reply-To: <00fd01d256fc$2f780db0$8e682910$@acksys.fr>
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.
> 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.
> 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.
--
Jouni Malinen PGP id EFC895FA
^ permalink raw reply
* [PATCH 3/3] nfc: trf7970a: Prevent repeated polling from crashing the kernel
From: Geoff Lansberry @ 2016-12-15 22:30 UTC (permalink / raw)
To: linux-wireless
Cc: lauro.venancio, aloisio.almeida, sameo, robh+dt, mark.rutland,
netdev, devicetree, linux-kernel, mgreer, justin, Jaret Cantu,
Geoff Lansberry
In-Reply-To: <1481841044-4314-1-git-send-email-glansberry@gmail.com>
From: Jaret Cantu <jaret.cantu@timesys.com>
Repeated polling attempts cause a NULL dereference error to occur.
This is because the curent state of the trf7970a is reading but
a request has been made to send a command.
The solution is to properly kill the waiting reading (workqueue)
before failing on the send.
---
drivers/nfc/trf7970a.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index b4c37ab..f96a321 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -1493,6 +1493,10 @@ static int trf7970a_send_cmd(struct nfc_digital_dev *ddev,
(trf->state != TRF7970A_ST_IDLE_RX_BLOCKED)) {
dev_err(trf->dev, "%s - Bogus state: %d\n", __func__,
trf->state);
+ if (trf->state == TRF7970A_ST_WAIT_FOR_RX_DATA ||
+ trf->state == TRF7970A_ST_WAIT_FOR_RX_DATA_CONT)
+ trf->ignore_timeout =
+ !cancel_delayed_work(&trf->timeout_work);
ret = -EIO;
goto out_err;
}
--
Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
^ permalink raw reply related
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