Re: About adding support for MT76x2U to Linux kernel
On Fri, Aug 14, 2015 at 2:32 PM, Jakub Kiciński wrote: > CC: Linus W who was hacking on mt7630e recently. FWIW I have pushed my git tree here: https://github.com/linusw/linux-mt7630e The driver codedrop from Mediatek turned out to be based on kernel v3.10 so there the patches applied cleanly. I then rebased that to v3.11 all the way up to v4.3. Branches are named: mt7630e- That branch does allow me to see access points so it's not totally worthless, but it doesn't associate or connect so there is logic missing, and that's where my knowledge of wireless fell short. It wasn't about git rebasing and simple C code anymore ... I actually had to know what I was doing, haha :) Since then I have not had time to go back and look at it again. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/RFT 1/2] mac80211: Add NEED_ALIGNED4_SKBS hw flag
HW/driver should set NEED_ALIGNED4_SKBS flag in case require aligned skbs to four-byte boundaries. Before we have to do memmove() in the driver before pass this to HW and memmove() back in tx completion. This patch allow to save CPU and skip such memmoves. For each skb we called memmove(ieee80211_hdrsize()) twice. Currently this was tested with ath9k, both hw/sw crypt for tkip/ccmp. For sure more tests required. Signed-off-by: Janusz Dziedzic --- include/net/mac80211.h | 4 net/mac80211/debugfs.c | 1 + net/mac80211/tkip.c| 15 --- net/mac80211/tx.c | 21 +++-- net/mac80211/wep.c | 6 ++ net/mac80211/wpa.c | 35 +++ 6 files changed, 69 insertions(+), 13 deletions(-) diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 7c30faf..0ea9b51 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -1910,6 +1910,9 @@ struct ieee80211_txq { * by just its MAC address; this prevents, for example, the same station * from connecting to two virtual AP interfaces at the same time. * + * @IEEE80211_HW_NEEDS_ALIGNED4_SKBS: Driver need aligned skbs to four-byte. + * Padding will be added after ieee80211_hdr. + * * @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays */ enum ieee80211_hw_flags { @@ -1946,6 +1949,7 @@ enum ieee80211_hw_flags { IEEE80211_HW_SUPPORTS_AMSDU_IN_AMPDU, IEEE80211_HW_BEACON_TX_STATUS, IEEE80211_HW_NEEDS_UNIQUE_STA_ADDR, + IEEE80211_HW_NEEDS_ALIGNED4_SKBS, /* keep last, obviously */ NUM_IEEE80211_HW_FLAGS diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c index abbdff0..fd45830 100644 --- a/net/mac80211/debugfs.c +++ b/net/mac80211/debugfs.c @@ -126,6 +126,7 @@ static const char *hw_flag_names[NUM_IEEE80211_HW_FLAGS + 1] = { FLAG(SUPPORTS_AMSDU_IN_AMPDU), FLAG(BEACON_TX_STATUS), FLAG(NEEDS_UNIQUE_STA_ADDR), + FLAG(NEEDS_ALIGNED4_SKBS), /* keep last for the build bug below */ (void *)0x1 diff --git a/net/mac80211/tkip.c b/net/mac80211/tkip.c index 0ae2077..26b2663 100644 --- a/net/mac80211/tkip.c +++ b/net/mac80211/tkip.c @@ -204,9 +204,18 @@ void ieee80211_get_tkip_p2k(struct ieee80211_key_conf *keyconf, const u8 *tk = &key->conf.key[NL80211_TKIP_DATA_OFFSET_ENCR_KEY]; struct tkip_ctx *ctx = &key->u.tkip.tx; struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data; - const u8 *data = (u8 *)hdr + ieee80211_hdrlen(hdr->frame_control); - u32 iv32 = get_unaligned_le32(&data[4]); - u16 iv16 = data[2] | (data[0] << 8); + unsigned int hdrlen; + const u8 *data; + u32 iv32; + u16 iv16; + + hdrlen = ieee80211_hdrlen(hdr->frame_control); + if (ieee80211_hw_check(&key->local->hw, NEEDS_ALIGNED4_SKBS)) + hdrlen += hdrlen & 3; + + data = (u8 *)hdr + hdrlen; + iv32 = get_unaligned_le32(&data[4]); + iv16 = data[2] | (data[0] << 8); spin_lock(&key->u.tkip.txlock); ieee80211_compute_tkip_p1k(key, iv32); diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 3311ce0..30ee9ad 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -937,6 +937,8 @@ ieee80211_tx_h_fragment(struct ieee80211_tx_data *tx) return TX_DROP; hdrlen = ieee80211_hdrlen(hdr->frame_control); + if (ieee80211_hw_check(&tx->local->hw, NEEDS_ALIGNED4_SKBS)) + hdrlen += hdrlen & 3; /* internal error, why isn't DONTFRAG set? */ if (WARN_ON(skb->len + FCS_LEN <= frag_threshold)) @@ -1796,6 +1798,8 @@ netdev_tx_t ieee80211_monitor_start_xmit(struct sk_buff *skb, hdr = (struct ieee80211_hdr *)(skb->data + len_rthdr); hdrlen = ieee80211_hdrlen(hdr->frame_control); + if (ieee80211_hw_check(&local->hw, NEEDS_ALIGNED4_SKBS)) + hdrlen += hdrlen & 3; if (skb->len < len_rthdr + hdrlen) goto fail; @@ -2020,6 +2024,7 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata, struct ieee80211_chanctx_conf *chanctx_conf; struct ieee80211_sub_if_data *ap_sdata; enum ieee80211_band band; + int padsize = 0; int ret; if (IS_ERR(sta)) @@ -2237,6 +2242,10 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata, hdrlen += 2; } + /* Check if HW require skb to be aligned */ + if (ieee80211_hw_check(&sdata->local->hw, NEEDS_ALIGNED4_SKBS)) + padsize = hdrlen & 3; + /* * Drop unicast frames to unauthorised stations unless they are * EAPOL frames from the local station. @@ -2323,6 +2332,7 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata, h_pos -= skip_header_bytes; head_need = hdrlen + encaps_len + meshhdrlen - skb_headroom(skb
[RFC/RFT 2/2] ath9k: request aligned skb
Set NEEDS_ALIGNED4_SKB hw flag. This allow driver to save CPU and remove two memmove from tx path. Signed-off-by: Janusz Dziedzic --- drivers/net/wireless/ath/ath9k/init.c | 1 + drivers/net/wireless/ath/ath9k/xmit.c | 15 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c index 6abace6..d578a00 100644 --- a/drivers/net/wireless/ath/ath9k/init.c +++ b/drivers/net/wireless/ath/ath9k/init.c @@ -831,6 +831,7 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw) ieee80211_hw_set(hw, RX_INCLUDES_FCS); ieee80211_hw_set(hw, HOST_BROADCAST_PS_BUFFERING); ieee80211_hw_set(hw, SUPPORT_FAST_XMIT); + ieee80211_hw_set(hw, NEEDS_ALIGNED4_SKBS); if (ath9k_ps_enable) ieee80211_hw_set(hw, SUPPORTS_PS); diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index 82fc76f..c3bd1b1 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -2267,11 +2267,15 @@ static int ath_tx_prepare(struct ieee80211_hw *hw, struct sk_buff *skb, padpos = ieee80211_hdrlen(hdr->frame_control); padsize = padpos & 3; if (padsize && skb->len > padpos) { - if (skb_headroom(skb) < padsize) - return -ENOMEM; + if (ieee80211_hw_check(hw, NEEDS_ALIGNED4_SKBS)) { + frmlen -= padsize; + } else { + if (skb_headroom(skb) < padsize) + return -ENOMEM; - skb_push(skb, padsize); - memmove(skb->data, skb->data + padsize, padpos); + skb_push(skb, padsize); + memmove(skb->data, skb->data + padsize, padpos); + } } setup_frame_info(hw, sta, skb, frmlen); @@ -2494,7 +2498,8 @@ static void ath_tx_complete(struct ath_softc *sc, struct sk_buff *skb, padpos = ieee80211_hdrlen(hdr->frame_control); padsize = padpos & 3; - if (padsize && skb->len>padpos+padsize) { + if (padsize && skb->len > padpos + padsize && + !ieee80211_hw_check(sc->hw, NEEDS_ALIGNED4_SKBS)) { /* * Remove MAC header padding before giving the frame back to * mac80211. -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/RFT 1/2] mac80211: Add NEED_ALIGNED4_SKBS hw flag
On 2015-12-17 10:20, Janusz Dziedzic wrote: > HW/driver should set NEED_ALIGNED4_SKBS flag in case require > aligned skbs to four-byte boundaries. > > Before we have to do memmove() in the driver before > pass this to HW and memmove() back in tx completion. > This patch allow to save CPU and skip such memmoves. > For each skb we called memmove(ieee80211_hdrsize()) twice. > > Currently this was tested with ath9k, both hw/sw crypt for > tkip/ccmp. > For sure more tests required. > > Signed-off-by: Janusz Dziedzic Nice. By the way, this alignment requirement is not ath9k specific - mt76 (currently out-of-tree), mt7601u and rt2x00 can use this as well. - Felix -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/13] ath9k: MCC add sta_ap_ratio module param
On 11 December 2015 at 09:19, Kalle Valo wrote: > Janusz Dziedzic writes: > >> In case of MCC we can setup STA/AP(GO) ratio. >> Eg. setting sta_ap_ratio=80 >> STA will get 80% of time, while AP(GO) 20%. >> Setup correct ctwindow. >> >> Signed-off-by: Janusz Dziedzic > > Why? What's the use case? > By default beacon_int/2 is used in current implementation. This patch was developed as a proof of concept in case we would like to use MCC and need change this 50/50. Eg, for some reason we need higher BW/ more air time for STA or AP - depends on case. For my case STA connection to Gateway was more important that clients connected to an AP - so just used 70/30. > And isn't there a better way to do this? Like using nl80211 (via > wpasupplicant?) or debugfs? > I wasn't sure here, but maybe some prio param on the VIF could be used here? Eg. VIF1 - prio 50 VIF2 - prio 100 VIF1_time = 50 / (50 + 100) = 33% VIF2_time = 100 / (50 + 100) = 66% with some default prio eg. 100. But for sure this is more work, and I am not sure someone else will need this? BR Janusz > -- > Kalle Valo -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/RFT 1/2] mac80211: Add NEED_ALIGNED4_SKBS hw flag
On Thu, 2015-12-17 at 10:20 +0100, Janusz Dziedzic wrote: > HW/driver should set NEED_ALIGNED4_SKBS flag in case require > aligned skbs to four-byte boundaries. > > Before we have to do memmove() in the driver before > pass this to HW and memmove() back in tx completion. > This patch allow to save CPU and skip such memmoves. > For each skb we called memmove(ieee80211_hdrsize()) twice. IMHO this is pretty awful from a code complexity POV. You also forgot to update fast-xmit maximum header length. Note that we (iwlwifi) also kinda need this, but essentially solve it with the DMA engine. Can't ath9k do the same? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/RFT 1/2] mac80211: Add NEED_ALIGNED4_SKBS hw flag
On 2015-12-17 11:04, Johannes Berg wrote: > On Thu, 2015-12-17 at 10:20 +0100, Janusz Dziedzic wrote: >> HW/driver should set NEED_ALIGNED4_SKBS flag in case require >> aligned skbs to four-byte boundaries. >> >> Before we have to do memmove() in the driver before >> pass this to HW and memmove() back in tx completion. >> This patch allow to save CPU and skip such memmoves. >> For each skb we called memmove(ieee80211_hdrsize()) twice. > > IMHO this is pretty awful from a code complexity POV. You also forgot > to update fast-xmit maximum header length. > > Note that we (iwlwifi) also kinda need this, but essentially solve it > with the DMA engine. Can't ath9k do the same? I tried that approach a few years ago, but it turned out to make the hardware unstable, causing random DMA lockups. - Felix -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/RFT 1/2] mac80211: Add NEED_ALIGNED4_SKBS hw flag
On 2015-12-17 10:20, Janusz Dziedzic wrote: > HW/driver should set NEED_ALIGNED4_SKBS flag in case require > aligned skbs to four-byte boundaries. > > Before we have to do memmove() in the driver before > pass this to HW and memmove() back in tx completion. > This patch allow to save CPU and skip such memmoves. > For each skb we called memmove(ieee80211_hdrsize()) twice. > > Currently this was tested with ath9k, both hw/sw crypt for > tkip/ccmp. > For sure more tests required. > > Signed-off-by: Janusz Dziedzic > --- > include/net/mac80211.h | 4 > net/mac80211/debugfs.c | 1 + > net/mac80211/tkip.c| 15 --- > net/mac80211/tx.c | 21 +++-- > net/mac80211/wep.c | 6 ++ > net/mac80211/wpa.c | 35 +++ > 6 files changed, 69 insertions(+), 13 deletions(-) > > diff --git a/net/mac80211/tkip.c b/net/mac80211/tkip.c > index 0ae2077..26b2663 100644 > --- a/net/mac80211/tkip.c > +++ b/net/mac80211/tkip.c > @@ -204,9 +204,18 @@ void ieee80211_get_tkip_p2k(struct ieee80211_key_conf > *keyconf, > const u8 *tk = &key->conf.key[NL80211_TKIP_DATA_OFFSET_ENCR_KEY]; > struct tkip_ctx *ctx = &key->u.tkip.tx; > struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data; > - const u8 *data = (u8 *)hdr + ieee80211_hdrlen(hdr->frame_control); > - u32 iv32 = get_unaligned_le32(&data[4]); > - u16 iv16 = data[2] | (data[0] << 8); > + unsigned int hdrlen; > + const u8 *data; > + u32 iv32; > + u16 iv16; > + > + hdrlen = ieee80211_hdrlen(hdr->frame_control); > + if (ieee80211_hw_check(&key->local->hw, NEEDS_ALIGNED4_SKBS)) > + hdrlen += hdrlen & 3; I think this check is duplicated way too often, maybe you should implement a wrapper for ieee80211_hdrlen and convert all relevant call sites. Makes it easier to spot places where this was forgotten. - Felix -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/RFT 1/2] mac80211: Add NEED_ALIGNED4_SKBS hw flag
On Thu, 2015-12-17 at 10:20 +0100, Janusz Dziedzic wrote: > HW/driver should set NEED_ALIGNED4_SKBS flag in case require > aligned skbs to four-byte boundaries. > > Before we have to do memmove() in the driver before > pass this to HW and memmove() back in tx completion. > This patch allow to save CPU and skip such memmoves. Can you quantify that btw? It shouldn't be that expensive since it's all in the cache for read, so just has to be written out to RAM before the DMA can happen... And on status it will be pulled into the cache anyway, and you don't even need the write to happen before you can continue. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/RFT 1/2] mac80211: Add NEED_ALIGNED4_SKBS hw flag
On 2015-12-17 11:29, Johannes Berg wrote: > On Thu, 2015-12-17 at 10:20 +0100, Janusz Dziedzic wrote: >> HW/driver should set NEED_ALIGNED4_SKBS flag in case require >> aligned skbs to four-byte boundaries. >> >> Before we have to do memmove() in the driver before >> pass this to HW and memmove() back in tx completion. >> This patch allow to save CPU and skip such memmoves. > > Can you quantify that btw? It shouldn't be that expensive since it's > all in the cache for read, so just has to be written out to RAM before > the DMA can happen... On many devices that I'm using, the data path is definitely too bloated for the packet to still be in cache, so I do think this will help. Remember, having only 32 KiB Dcache (no L2) is not uncommon on devices running ath9k. > And on status it will be pulled into the cache > anyway, and you don't even need the write to happen before you can > continue. This doesn't make sense to me. The write needs to happen before the device can do DMA... - Felix -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/RFT 1/2] mac80211: Add NEED_ALIGNED4_SKBS hw flag
On Thu, 2015-12-17 at 11:35 +0100, Felix Fietkau wrote: > > On many devices that I'm using, the data path is definitely too > bloated > for the packet to still be in cache, so I do think this will help. > Remember, having only 32 KiB Dcache (no L2) is not uncommon on > devices > running ath9k. Hm, ok :) > > And on status it will be pulled into the cache > > anyway, and you don't even need the write to happen before you can > > continue. > This doesn't make sense to me. The write needs to happen before the > device can do DMA... > On *status* :) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Print warnings for missing cfg80211_ops implementations
On Thu, 2015-12-17 at 08:57 +0100, Johannes Berg wrote: > On Thu, 2015-12-17 at 08:34 +0100, Ola Olsson wrote: > > > but maybe it should be > > > > > > WARN_ON((ops->add_station && !ops->del_station) || > > > (!opt->add_station && ops->del_station)) > > > > > > etc... > > > > Ahh, got it! I really like your idea but I assume it's quite rare to > > implement the "stop/del/leave/disconnect" callbacks and at the same > > time forget to implement the "start/add/join/connect". You will > > probably find out pretty quickly if the "start" functions are > > missing, > > while it might take some time debugging why you lack the "stop" > > functions (reinitialization issues/ resource leaks for example). > > > > With that said, don't take my word for it, I was only following the > > existing pattern and I assume someone else had a good reason in the > > first place. > > > > Pretty much what you said :) Following patterns is good, I just think the pattern could be trivially improved. The test is a runtime check on what would ideally be done at compile time. Using WARN_ON(!a ^ !b) which is logically the same as what I wrote above for clarity is simply a bit more coverage and maybe even a bit run-time faster. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Print warnings for missing cfg80211_ops implementations
> Using > WARN_ON(!a ^ !b) > which is logically the same as what I wrote above > for clarity is simply a bit more coverage and maybe > even a bit run-time faster. > > Didn't think about that. Love your bit fiddling trick! After an approval/rejection of my patch I can submit another one with your suggestion unless you want to do it yourself... /Ola -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Print warnings for missing cfg80211_ops implementations
On Thu, 2015-12-17 at 03:01 -0800, Joe Perches wrote: > Following patterns is good, I just think the > pattern could be trivially improved. It's a question of what makes sense though - nobody implements stop_xyz without implementing start_xyz, and even if they do it's pointless. It's just that if you have start_xyz most/all of your functional tests might work, but we'd really like to have stop_xyz as well. It's not *worse* to check for the XOR (like you suggest below), but it's not really any better either. > The test is a runtime check on what would ideally > be done at compile time. If you have any suggestions how to do that then that'd be great :) I don't really see a way of doing that since this depends on the driver and the driver might even fill the struct at runtime (like hwsim does IIRC) > Using > WARN_ON(!a ^ !b) > which is logically the same as what I wrote above > for clarity is simply a bit more coverage and maybe > even a bit run-time faster. Don't think we have to worry much about the runtime overhead, but that's a nice idea. As I said above though, I don't think it really makes a difference. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pull-request: mac80211 2015-12-15
On Wed, 2015-12-16 at 18:34 -0500, David Miller wrote: > > Something about your text encoding kept this from ending up > in patchwork for some reason. > Hm. I don't see anything special with this, seems to just be plain text 8bit transfer encoding. Do you want me to watch out for things getting into patchwork in the future? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: question on "mac80211_hwsim: support any address in userspace"
On Wed, Dec 16, 2015 at 02:14:18PM -0800, Ben Greear wrote: > On 12/16/2015 09:30 AM, Adam R. Welle wrote: > > > >>Well, it was always rather awkward since it was the *second* address :) > > > >Could somebody provide background information on why the decision was > >made to use a second address for the netlink frames instead of the same > >address as was used for the non-netlink frames? > > I would be fine with always using the first address instead of the second, > in case that helps someone. It doesn't really, that would break old wmediumd. > We could also set the address at creation time easily enough. Then it > could still be unique across many machines if you managed it. We already have a way to change mac addresses; adding a new API to change the default address seems pointless to me. -- Bob Copeland %% http://bobcopeland.com/ -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/1] staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration
From: Anatoly Stepanov Hi, Jes! I updated the patch, according to your idea, now "param" is passed by value. Anatoly Stepanov (1): staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration drivers/staging/rtl8723au/hal/odm.c | 2 +- drivers/staging/rtl8723au/hal/rtl8723a_cmd.c | 6 +++--- drivers/staging/rtl8723au/include/rtl8723a_cmd.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/1] staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration
From: Anatoly Stepanov Previosly the function had the following prototype: int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param) My suggestion here is to modify the prototype this way: int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param) We can do this based on the following considerations: 1. rtl8723a_set_rssi_cmd is used only with 32-bit "param" values 2. There's no point in using "u8 *param" until we pass param length 3. As we just read "param", it's ok to pass it by value Signed-off-by: Anatoly Stepanov --- drivers/staging/rtl8723au/hal/odm.c | 2 +- drivers/staging/rtl8723au/hal/rtl8723a_cmd.c | 6 +++--- drivers/staging/rtl8723au/include/rtl8723a_cmd.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/rtl8723au/hal/odm.c b/drivers/staging/rtl8723au/hal/odm.c index 6b9dbef..45d70fd 100644 --- a/drivers/staging/rtl8723au/hal/odm.c +++ b/drivers/staging/rtl8723au/hal/odm.c @@ -1274,7 +1274,7 @@ static void odm_RSSIMonitorCheck(struct dm_odm_t *pDM_Odm) for (i = 0; i < sta_cnt; i++) { if (PWDB_rssi[i] != (0)) - rtl8723a_set_rssi_cmd(Adapter, (u8 *)&PWDB_rssi[i]); + rtl8723a_set_rssi_cmd(Adapter, PWDB_rssi[i]); } pdmpriv->EntryMaxUndecoratedSmoothedPWDB = MaxDB; diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c index 1662c03c..2230f4c 100644 --- a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c +++ b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c @@ -113,11 +113,11 @@ exit: return ret; } -int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param) +int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param) { - *((u32 *)param) = cpu_to_le32(*((u32 *)param)); + __le32 cmd = cpu_to_le32(param); - FillH2CCmd(padapter, RSSI_SETTING_EID, 3, param); + FillH2CCmd(padapter, RSSI_SETTING_EID, 3, (void *)&cmd); return _SUCCESS; } diff --git a/drivers/staging/rtl8723au/include/rtl8723a_cmd.h b/drivers/staging/rtl8723au/include/rtl8723a_cmd.h index 014c02e..f95535a 100644 --- a/drivers/staging/rtl8723au/include/rtl8723a_cmd.h +++ b/drivers/staging/rtl8723au/include/rtl8723a_cmd.h @@ -149,7 +149,7 @@ void rtl8723a_set_BTCoex_AP_mode_FwRsvdPkt_cmd(struct rtw_adapter *padapter); #else #define rtl8723a_set_BTCoex_AP_mode_FwRsvdPkt_cmd(padapter) do {} while(0) #endif -int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param); +int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param); int rtl8723a_set_raid_cmd(struct rtw_adapter *padapter, u32 mask, u8 arg); void rtl8723a_add_rateatid(struct rtw_adapter *padapter, u32 bitmap, u8 arg, u8 rssi_level); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] staging: rtl8723au: change parameter type in rtl8723a_set_rssi_cmd declaration
drivengro...@gmail.com writes: > From: Anatoly Stepanov > > Previosly the function had the following prototype: > int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param) > > My suggestion here is to modify the prototype this way: > int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param) > > We can do this based on the following considerations: > 1. rtl8723a_set_rssi_cmd is used only with 32-bit "param" values > 2. There's no point in using "u8 *param" until we pass param length > 3. As we just read "param", it's ok to pass it by value > > Signed-off-by: Anatoly Stepanov > --- > drivers/staging/rtl8723au/hal/odm.c | 2 +- > drivers/staging/rtl8723au/hal/rtl8723a_cmd.c | 6 +++--- > drivers/staging/rtl8723au/include/rtl8723a_cmd.h | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) Looks good to me! Acked-by: Jes Sorensen > > diff --git a/drivers/staging/rtl8723au/hal/odm.c > b/drivers/staging/rtl8723au/hal/odm.c > index 6b9dbef..45d70fd 100644 > --- a/drivers/staging/rtl8723au/hal/odm.c > +++ b/drivers/staging/rtl8723au/hal/odm.c > @@ -1274,7 +1274,7 @@ static void odm_RSSIMonitorCheck(struct dm_odm_t > *pDM_Odm) > > for (i = 0; i < sta_cnt; i++) { > if (PWDB_rssi[i] != (0)) > - rtl8723a_set_rssi_cmd(Adapter, (u8 *)&PWDB_rssi[i]); > + rtl8723a_set_rssi_cmd(Adapter, PWDB_rssi[i]); > } > > pdmpriv->EntryMaxUndecoratedSmoothedPWDB = MaxDB; > diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c > b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c > index 1662c03c..2230f4c 100644 > --- a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c > +++ b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c > @@ -113,11 +113,11 @@ exit: > return ret; > } > > -int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param) > +int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param) > { > - *((u32 *)param) = cpu_to_le32(*((u32 *)param)); > + __le32 cmd = cpu_to_le32(param); > > - FillH2CCmd(padapter, RSSI_SETTING_EID, 3, param); > + FillH2CCmd(padapter, RSSI_SETTING_EID, 3, (void *)&cmd); > > return _SUCCESS; > } > diff --git a/drivers/staging/rtl8723au/include/rtl8723a_cmd.h > b/drivers/staging/rtl8723au/include/rtl8723a_cmd.h > index 014c02e..f95535a 100644 > --- a/drivers/staging/rtl8723au/include/rtl8723a_cmd.h > +++ b/drivers/staging/rtl8723au/include/rtl8723a_cmd.h > @@ -149,7 +149,7 @@ void rtl8723a_set_BTCoex_AP_mode_FwRsvdPkt_cmd(struct > rtw_adapter *padapter); > #else > #define rtl8723a_set_BTCoex_AP_mode_FwRsvdPkt_cmd(padapter) do {} while(0) > #endif > -int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param); > +int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u32 param); > int rtl8723a_set_raid_cmd(struct rtw_adapter *padapter, u32 mask, u8 arg); > void rtl8723a_add_rateatid(struct rtw_adapter *padapter, u32 bitmap, u8 arg, > u8 rssi_level); -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] mac80211: improve the contiguous mask checking
If the result of adding the first set bit to the mask is power of 2, the mask must be contiguous. "mask & -mask" can get the first set bit of mask gracefully. Signed-off-by: Zeng Zhaoxiu --- net/mac80211/iface.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index c9e325d..4c896e8 100644 --- a/net/mac80211/iface.c +++ b/net/mac80211/iface.c @@ -1628,7 +1628,9 @@ static void ieee80211_assign_perm_addr(struct ieee80211_local *local, ((u64)m[2] << 3*8) | ((u64)m[3] << 2*8) | ((u64)m[4] << 1*8) | ((u64)m[5] << 0*8); - if (__ffs64(mask) + hweight64(mask) != fls64(mask)) { + inc = (mask & -mask); + val = mask + inc; + if ((val & (val - 1)) != 0) { /* not a contiguous mask ... not handled now! */ pr_info("not contiguous\n"); break; @@ -1649,7 +1651,6 @@ static void ieee80211_assign_perm_addr(struct ieee80211_local *local, ((u64)m[2] << 3*8) | ((u64)m[3] << 2*8) | ((u64)m[4] << 1*8) | ((u64)m[5] << 0*8); - inc = 1ULL<<__ffs64(mask); val = (start & mask); addr = (start & ~mask) | (val & mask); do { -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] mac80211: improve the contiguous mask checking
On Thu, 2015-12-17 at 22:59 +0800, Zeng Zhaoxiu wrote: > If the result of adding the first set bit to the mask is power of 2, > the mask must be contiguous. "mask & -mask" can get the first set bit > of mask gracefully. > - if (__ffs64(mask) + hweight64(mask) != fls64(mask)) > { > + inc = (mask & -mask); > + val = mask + inc; > + if ((val & (val - 1)) != 0) { > /* not a contiguous mask ... not handled now! */ Hm. Ok, I can see how that would work, but it doesn't really seem like much of an "improvement" to me? Surely I seem to need much more thinking to understand this. There's no reason to optimise it either, so why should we change it? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] New location for wireless-testing tree
On Wed, Dec 16, 2015 at 12:32:23PM -0500, Bob Copeland wrote: > As reported previously[1], John Linville will be moving on from maintenance > of the wireless-testing[2] tree at the end of the year. A huge thank you to > John for doing all this work for so many years! > > We now have a shared wireless directory on kernel.org where we (currently > myself and Kalle as backup) will continue to maintain this tree going > forward. The new tree can be found at: > > git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-testing.git > > Note: unlike John's tree, we are going to try rebasing this tree on every > build (like linux-next) instead of merging the downstream trees, in order > to avoid certain merge problems when those trees are rebased. > > This means that if you are working directly off of wireless-testing, use > 'git pull --rebase' to sync, instead of just 'git pull'. > > We'll see how this goes and reassess after a couple of cycles. > > Please let me know of any issues. > > [1] http://comments.gmane.org/gmane.linux.kernel.wireless.general/145291 > > [2] wireless-testing is an integration testing tree, consisting of: > > * Linus's latest -rc > * patches in mac80211 and wireless-drivers (for the upcoming release) > * patches in mac80211-next and wireless-drivers-next (for the next release) > > It is not pulled into any upstream tree, but it should be a pretty good > indication of what is baking in Linux wireless for the next release, without > having unrelated changes from all the other subsystems as in linux-next. Given this most welcome announcement, I will hereby cease maintenance of the predecessor wireless-testing tree in favor of this new canonical source. Thanks guys! John -- John W. LinvilleSomeday the world will need a hero, and you linvi...@tuxdriver.com might be all we have. Be ready. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pull-request: mac80211 2015-12-15
From: Johannes Berg Date: Thu, 17 Dec 2015 13:44:32 +0100 > On Wed, 2015-12-16 at 18:34 -0500, David Miller wrote: >> >> Something about your text encoding kept this from ending up >> in patchwork for some reason. >> > > Hm. I don't see anything special with this, seems to just be plain text > 8bit transfer encoding. > > Do you want me to watch out for things getting into patchwork in the > future? Look in the quoted text of mine, see that underline thing after the ">>"? Where are those coming from? Those were all over the place in your pull request and tripped up patchwork's parser I guess. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] New location for wireless-testing tree
On Thu, Dec 17, 2015 at 5:56 PM, John W. Linville wrote: > On Wed, Dec 16, 2015 at 12:32:23PM -0500, Bob Copeland wrote: >> As reported previously[1], John Linville will be moving on from maintenance >> of the wireless-testing[2] tree at the end of the year. A huge thank you to >> John for doing all this work for so many years! >> >> We now have a shared wireless directory on kernel.org where we (currently >> myself and Kalle as backup) will continue to maintain this tree going >> forward. The new tree can be found at: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-testing.git >> >> Note: unlike John's tree, we are going to try rebasing this tree on every >> build (like linux-next) instead of merging the downstream trees, in order >> to avoid certain merge problems when those trees are rebased. >> >> This means that if you are working directly off of wireless-testing, use >> 'git pull --rebase' to sync, instead of just 'git pull'. >> >> We'll see how this goes and reassess after a couple of cycles. >> >> Please let me know of any issues. >> >> [1] http://comments.gmane.org/gmane.linux.kernel.wireless.general/145291 >> >> [2] wireless-testing is an integration testing tree, consisting of: >> >> * Linus's latest -rc >> * patches in mac80211 and wireless-drivers (for the upcoming release) >> * patches in mac80211-next and wireless-drivers-next (for the next release) >> >> It is not pulled into any upstream tree, but it should be a pretty good >> indication of what is baking in Linux wireless for the next release, without >> having unrelated changes from all the other subsystems as in linux-next. > > Given this most welcome announcement, I will hereby cease maintenance > of the predecessor wireless-testing tree in favor of this new > canonical source. > Somewhat related. I started a while ago to mirror our internal development tree to korg. You'll find there the bleeding edge of iwlwifi's development: both fixes and new features. It is a backport based tree. Pretty much all the patches for mac80211{-next}.git that we (Intel) send come from there as well. Note that this is the "raw data" (including Change-IDs and server that are inside Intel's intranet) which I later edit to send patches upstream. I merge mac80211.git and mac80211-next.git to that tree to get mac80211 patches that come from upstream. In a sense, this tree looks like a backport of wireless-testing with content that is not there yet. You'll also find there "stable branches" that go through more thorough test cycles against their matching firmware version. I am not sure anyone is interested, but if someone wants, the tree is here: https://git.kernel.org/cgit/linux/kernel/git/iwlwifi/backport-iwlwifi.git/ A bit more info on the "stable branches": https://wireless.wiki.kernel.org/en/users/drivers/iwlwifi/core_release Drop me an email if you start to use this tree just to know if people are really using it :) -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pull-request: mac80211 2015-12-15
On Thu, 17 Dec 2015 12:04:48 -0500 (EST) David Miller wrote: > From: Johannes Berg > Date: Thu, 17 Dec 2015 13:44:32 +0100 > > > On Wed, 2015-12-16 at 18:34 -0500, David Miller wrote: > >> > >> Something about your text encoding kept this from ending up > >> in patchwork for some reason. > >> > > > > Hm. I don't see anything special with this, seems to just be plain > > text 8bit transfer encoding. > > > > Do you want me to watch out for things getting into patchwork in the > > future? > > Look in the quoted text of mine, see that underline thing after > the ">>"? Where are those coming from? Those were all over the > place in your pull request and tripped up patchwork's parser I > guess. I was curious and took a look. I suspect what you're seeing are the UTF-8 <0xC2 0xA0> sequence, which translates to codepoint 0x00A0 "no-break space" (same as in latin1). They seem to have been used in place of regular spaces for the purpose of indenting. The Content-Type charset in Johannes' emails is "UTF-8", so I think that's legal. Although I have no idea how Patchwork reacts to it. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pull-request: mac80211 2015-12-15
On Thu, 2015-12-17 at 13:44 +0100, Johannes Berg wrote: > On Wed, 2015-12-16 at 18:34 -0500, David Miller wrote: > > > > Something about your text encoding kept this from ending up > > in patchwork for some reason. > > > > Hm. I don't see anything special with this, seems to just be plain > text > 8bit transfer encoding. > > Do you want me to watch out for things getting into patchwork in the > future? > Hey Johannes. You seem to be using: X-Mailer: Evolution 3.18.1-1 which, to be overly technical, _sucks_. The new composer for Evolution has way too many defects to list. It adds non-breaking spaces (NBSP) characters instead of a standard ASCII space in various places. Every version of Evolution since 3.14 is terrible at sending text only messages. 3.12 works -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems loading firmware using built-in drivers with kernels that use initramfs.
On Sun, Aug 30, 2015 at 11:11 AM, Linus Torvalds wrote: > On Sun, Aug 30, 2015 at 1:25 AM, Arend van Spriel wrote: >> On 08/29/2015 12:38 PM, Ming Lei wrote: >> >> Does this mean a built-in driver can not get firmware from initramfs or >> built in the kernel early. Seems a bit too aggressive. > > Yeah, that seems wrong. Loading firmware from initramfs is required > for some things, like disk drivers. Of course, depending on how it's > done, it's all after the SYSTEM_BOOTING phase, but .. > > What we *might* do is to not allow it for the user-mode helper > fallback, FWIW, that's what we did, request_firmware_direct() now skips the silly usermode helper. I'll note that Greg pointed out to me that Daniel (was this right?) might have some use cases for the usermode helper in the future on graphics though. Daniel is that right? Can you clarify the use case, I'd just like to document this and keep it in mind for future design changes on firmware_class. Also, it occurs to me that if you have a need for it, perhaps others might and if we can avoid *others* from coming up with their own solution that'd be best, specifically as this is related to file loading -- more on this later generalized possible use cases in another thread I'll Cc you folks on. > but I think it's more likely that we'll just deprecate the > usermode helper fw loader entirely, so adding new error cases for it > seems pointless. I was shooting hard to deprecate the usermodehelper, Greg has noted that we can only phase it out though, so that is what I will be shooting for: a sysdata API, what will have firmware signing support later will *not* make use of the usermode helper. The old APIs will still use it. [0] http://lkml.kernel.org/r/20151006090821.gb9...@kroah.com Luis -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/7] ima: load policy using path
The subject should probably be: ima: add support to load policy from path Cc'ing Roberts who originally wanted SELinux file policy signing capabilities. Also Greg, who is reviewing the sysdata file changes I had proposed which would provide a generic file loading facility for modules, and later a generic file signing checker. Cc'ing Linus for any feedback on the possible issues he might foresee if we all go gung-ho on "kernel file loading", as this is where it seems some folks might be going. I *think* the user hint might help close the semantic gap observed on using a file loader on firmware_class, but perhaps he or other might have some other corner cases in mind we should also consider. Note that the crux between using a generic kernel file loader and the common "sysdata" file loader will be that sysdata file users (wireless would be one getting rid of CRDA) and a core kernel file loader (IMA could be one, likewise perhaps SELinux if it follows suit in design as in this patch) is that the core kernel file loader would allow arbitrary file paths, and the sysdata users would have stuff in /lib/firmware/ paths (and therefore also have things in the linux-firmware tree). On Tue, Dec 08, 2015 at 01:01:20PM -0500, Mimi Zohar wrote: > From: Dmitry Kasatkin > > Currently the IMA policy is loaded by writing the policy rules to > '/ima/policy'. > That way the policy cannot be measured or appraised. This patch extends the > policy loading interface with the possibility to load the policy using a > pathname. The policy can be loaded like: > > echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy I don't think this is as clear, you can just say something like: - We currently cannot do appraisal or signature vetting of IMA policies since we currently can only load IMA policies by writing the contents of the polify directly in, as follows: cat policy-file > /ima/policy If we provide the kernel the path to the IMA policy so it can load the policy itself it'd be able to later appraise or vet for the file signature if it had one. This adds support to load IMA policies with a given path as follows: echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy - > > Signed-off-by: Dmitry Kasatkin > Signed-off-by: Mimi Zohar > --- > security/integrity/iint.c | 4 +--- > security/integrity/ima/ima_fs.c | 39 ++- > security/integrity/integrity.h | 2 +- > 3 files changed, 40 insertions(+), 5 deletions(-) > > diff --git a/security/integrity/iint.c b/security/integrity/iint.c > index 2de9c82..54b51a4 100644 > --- a/security/integrity/iint.c > +++ b/security/integrity/iint.c > @@ -203,10 +203,8 @@ int integrity_kernel_read(struct file *file, loff_t > offset, > * This is function opens a file, allocates the buffer of required > * size, read entire file content to the buffer and closes the file > * > - * It is used only by init code. > - * > */ > -int __init integrity_read_file(const char *path, char **data) > +int integrity_read_file(const char *path, char **data) > { > struct file *file; > loff_t size; > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > index eebb985..f902b6b 100644 > --- a/security/integrity/ima/ima_fs.c > +++ b/security/integrity/ima/ima_fs.c > @@ -258,6 +258,40 @@ static const struct file_operations > ima_ascii_measurements_ops = { > .release = seq_release, > }; > > +static ssize_t ima_read_policy(char *path) > +{ > + char *data, *datap; > + int rc, size, pathlen = strlen(path); > + char *p; > + > + /* remove \n */ > + datap = path; > + strsep(&datap, "\n"); > + > + rc = integrity_read_file(path, &data); > + if (rc < 0) > + return rc; > + > + size = rc; > + datap = data; > + > + while (size > 0 && (p = strsep(&datap, "\n"))) { > + pr_debug("rule: %s\n", p); > + rc = ima_parse_add_rule(p); > + if (rc < 0) > + break; > + size -= rc; > + } > + > + kfree(data); > + if (rc < 0) > + return rc; > + else if (size) > + return -EINVAL; > + else > + return pathlen; > +} > + Please no, instead of adding yet-another kernel file-loading facility which is likely error prone we should consolidate *all kernel file-loading facilities* into a *common generic shared one*. So please work to make that happen since you need yet-another user for it. I've done some initial homework already on a few prominent common users. I say "initial" homework as my search was rather crude on 'git grep' and not done using semantic parsers to see if I "search for file loaders" through semantic means. This later search may be optional, but it would help the hunt be more complete. I've listed in two separate thre
[PATCH] bcma: use module_init for the main part of bus initialization
So far we were using fs_initcall. It was (and still is) needed because struct bus_type has to be registered early. However main bus initialization has to happen later as it requires SPROM which depends on NVRAM which depends on mtd. Solve it by using fs_initcall only for bus_register call and module_init for the rest. It affects bcma only when built-in obviously. This was tested with BCM4706 and BCM5357C0 (BCM47XX), BCM4708A0 (ARCH_BCM_5301X) and BCM43225 (PCIe card with bcma as module). Signed-off-by: Rafał Miłecki --- This change was already suggested in RFC patch early this year: https://patchwork.kernel.org/patch/5802611/ Noone objected / got any better idea, so I'm sending a final version. V1 (from RFC): * Use int bcma_bus_registered to avoid #ifdef MODULE and make code simpler. * Document reason for this behavior better. --- drivers/bcma/main.c | 29 +++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c index 59d8d0d..c466f75 100644 --- a/drivers/bcma/main.c +++ b/drivers/bcma/main.c @@ -668,11 +668,36 @@ static int bcma_device_uevent(struct device *dev, struct kobj_uevent_env *env) core->id.rev, core->id.class); } -static int __init bcma_modinit(void) +static unsigned int bcma_bus_registered; + +/* + * If built-in, bus has to be registered early, before any driver calls + * bcma_driver_register. + * Otherwise registering driver would trigger BUG in driver_register. + */ +static int __init bcma_init_bus_register(void) { int err; + if (bcma_bus_registered) + return 0; + err = bus_register(&bcma_bus_type); + if (!err) + bcma_bus_registered = 1; + + return err; +} +#ifndef MODULE +fs_initcall(bcma_init_bus_register); +#endif + +/* Main initialization has to be done with SPI/mtd/NAND/SPROM available */ +static int __init bcma_modinit(void) +{ + int err; + + err = bcma_init_bus_register(); if (err) return err; @@ -691,7 +716,7 @@ static int __init bcma_modinit(void) return err; } -fs_initcall(bcma_modinit); +module_init(bcma_modinit); static void __exit bcma_modexit(void) { -- 1.8.4.5 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About adding support for MT76x2U to Linux kernel
Hi Felix and Jakub, On Wed, Dec 16, 2015 at 10:24:25PM +0100, Johannes Stezenbach wrote: > On Wed, Dec 16, 2015 at 07:53:04PM +0100, Felix Fietkau wrote: > > Once I'm done with mt7603 bringup (monitor mode rx already works), I > > plan on submitting mt76 upstream, and then we can see about adding > > support for 76x2u, or even merging the hardware support for mt7601u. > > > > You can check out my work in the mt7603 branch here: > > https://github.com/openwrt/mt76/tree/mt7603 > > let me chime in to say that I'm interested to work on > support for mt7610u (TP-LINK Archer T2U/T2UH). > https://wikidevi.com/wiki/TP-LINK_Archer_T2U > > I'm facing the same question as Tuomas about using > mt76 or mt7601u as the base? It's not like I didn't get from your earlier mail that you'd rather have us wait for mt76 to be upstream, but I guess that'll take half a year at least, right? Probably it would take me months to get something working anyway as I have a lot to learn and I'm doing it in my spare time. I have no problem to rewrite the code multiple times as needed when upstream changes. But I'd love to get your recommendation how to get started soon, and especially if you know if the mt7610u chipset is closer to mt7601u or mt76x[23][eu]? Thanks, Johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: add modparam 'hw_csum' to make HW checksum configurable
On 12/16/2015 11:29 PM, Michal Kazior wrote: On 17 December 2015 at 00:50, Peter Oh wrote: On 12/16/2015 01:54 PM, Felix Fietkau wrote: On 2015-12-16 22:19, Peter Oh wrote: [...] If mentioned to use the function to mesh frame only without touching mac80211, then how do you suggest it to apply it only to mesh frame without interfere other data frames? Can you share your example? It's trivial - in ath10k_tx you do this: if (vif->type == NL80211_IFTYPE_MESH_POINT && skb->ip_summed == CHECKSUM_PARTIAL) skb_checksum_help(skb); Thank you Felix for the quick response. I agree on your user experience opinion, but what do you think when ath10k has a new chip supporting HW checksum for Mesh? You can simply introduce a fw-feature flag saying "supports_mesh_csum_offload" later and skip the skb_checksum_help() if it's set. If we rely on fw-feature flag, then we are not able to use HW checksum at all even for AP/STA interfaces. Michał Thanks, Peter -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: add modparam 'hw_csum' to make HW checksum configurable
On 12/16/2015 03:59 PM, Felix Fietkau wrote: On 2015-12-17 00:50, Peter Oh wrote: On 12/16/2015 01:54 PM, Felix Fietkau wrote: On 2015-12-16 22:19, Peter Oh wrote: On 12/16/2015 12:53 PM, Felix Fietkau wrote: On 2015-12-16 21:46, Peter Oh wrote: On 12/16/2015 12:35 PM, Felix Fietkau wrote: On 2015-12-16 21:29, Peter Oh wrote: On 12/16/2015 10:27 AM, Felix Fietkau wrote: On 2015-12-16 19:20, Peter Oh wrote: Some hardwares such as QCA988X and QCA99X0 doesn't have capability of checksum offload when frame formats are not suitable for it such as Mesh frame. Hence add a module parameter, hw_csum, to make checksum offload configurable during module registration time. Signed-off-by: Peter Oh How about instead of inventing yet another crappy module parameter, you call skb_checksum_help() in the driver in cases where the hardware is unable to offload the checksum calculation. That way the user has to worry about less driver specific hackery ;) That will be good option for hardware not supporting HW checksum, but I mind that using the function will add more workload per every packet on critical data path when HW supports checksum resulting in throughput down. I didn't mean calling it for every single frame in the data path. What I'm suggesting is calling it selectively only for mesh frames, or any other frames that the hardware cannot offload, and leaving the rest for the hardware to process. There should be no performance difference between disabling checksum offload and calling skb_checksum_help from the driver. To call it selectively for Mesh frame or interface, we need to add it on mac80211 layer such as ieee80211_build_hdr() since driver layer does not care the interface type in data path. No need to change mac80211 - it only touches the headers, and skb_checksum_help does not care about that. The skb has enough information for it to find the right range to calculate the checksum and the place to store it. If mentioned to use the function to mesh frame only without touching mac80211, then how do you suggest it to apply it only to mesh frame without interfere other data frames? Can you share your example? It's trivial - in ath10k_tx you do this: if (vif->type == NL80211_IFTYPE_MESH_POINT && skb->ip_summed == CHECKSUM_PARTIAL) skb_checksum_help(skb); Thank you Felix for the quick response. I agree on your user experience opinion, but what do you think when ath10k has a new chip supporting HW checksum for Mesh? Then you simply update the checks. What's the big deal? keep adding condition to such data path is not a good option. I also considered again about user experiences and reached to that this patch won't disturb user experience since the products will ship with proper module settings. for instance the parameter will be turned on if product support it other wise will be turned off as they shipped, so that users don't need to touch it. In addition, for enterprise customers, they do care even a very small performance drop or enhancement especially when they are running BMT among vendors. So we need to avoid adding extra codes in data path in my opinion. - Felix -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: add modparam 'hw_csum' to make HW checksum configurable
On 2015-12-17 23:01, Peter Oh wrote: > > On 12/16/2015 03:59 PM, Felix Fietkau wrote: >> On 2015-12-17 00:50, Peter Oh wrote: >>> On 12/16/2015 01:54 PM, Felix Fietkau wrote: On 2015-12-16 22:19, Peter Oh wrote: > On 12/16/2015 12:53 PM, Felix Fietkau wrote: >> On 2015-12-16 21:46, Peter Oh wrote: >>> On 12/16/2015 12:35 PM, Felix Fietkau wrote: On 2015-12-16 21:29, Peter Oh wrote: > On 12/16/2015 10:27 AM, Felix Fietkau wrote: >> On 2015-12-16 19:20, Peter Oh wrote: >>> Some hardwares such as QCA988X and QCA99X0 doesn't have >>> capability of checksum offload when frame formats are not >>> suitable for it such as Mesh frame. >>> Hence add a module parameter, hw_csum, to make checksum offload >>> configurable during module registration time. >>> >>> Signed-off-by: Peter Oh >> How about instead of inventing yet another crappy module parameter, >> you >> call skb_checksum_help() in the driver in cases where the hardware is >> unable to offload the checksum calculation. >> >> That way the user has to worry about less driver specific hackery ;) > That will be good option for hardware not supporting HW checksum, but > I > mind that using the function will add more workload per every packet > on > critical data path when HW supports checksum resulting in throughput > down. I didn't mean calling it for every single frame in the data path. What I'm suggesting is calling it selectively only for mesh frames, or any other frames that the hardware cannot offload, and leaving the rest for the hardware to process. There should be no performance difference between disabling checksum offload and calling skb_checksum_help from the driver. >>> To call it selectively for Mesh frame or interface, we need to add it on >>> mac80211 layer such as ieee80211_build_hdr() since driver layer does not >>> care the interface type in data path. >> No need to change mac80211 - it only touches the headers, and >> skb_checksum_help does not care about that. The skb has enough >> information for it to find the right range to calculate the checksum and >> the place to store it. > If mentioned to use the function to mesh frame only without touching > mac80211, then how do you suggest it to apply it only to mesh frame > without interfere other data frames? > Can you share your example? It's trivial - in ath10k_tx you do this: if (vif->type == NL80211_IFTYPE_MESH_POINT && skb->ip_summed == CHECKSUM_PARTIAL) skb_checksum_help(skb); >>> Thank you Felix for the quick response. >>> I agree on your user experience opinion, >>> but what do you think when ath10k has a new chip supporting HW checksum >>> for Mesh? >> Then you simply update the checks. What's the big deal? > keep adding condition to such data path is not a good option. > I also considered again about user experiences and reached to that this > patch won't disturb user experience since the products will ship with > proper module settings. for instance the parameter will be turned on if > product support it other wise will be turned off as they shipped, so > that users don't need to touch it. I think the point you were missing is the one that there is no such thing as a proper setting for this module parameter, since it doesn't really depend much on the hardware or the product, but on the wifi mode that you are using. > In addition, for enterprise customers, they do care even a very small > performance drop or enhancement especially when they are running BMT > among vendors. > So we need to avoid adding extra codes in data path in my opinion. The regular data tx path already checks ar->dev_flags to decide whether to use raw mode or not. This means that this part of the data structure is already cached. The vif type is also cached, since it's accessed in the same part of the function. Because of that, the impact of adding an extra check even for a hardware capability will be so low, that I'm pretty sure you will not be able to measure it. And even if it were measurable, it's probably quite easy to find a few places to optimize I find the tradeoff you are making very odd: For users that don't know about the module parameter (depending on the default value) it either just randomly doesn't work in mesh or always runs with degraded performance. All this to save adding a check that will be completely irrelevant for performance, since it won't result in any extra cache stalls (which are the typical bottleneck in the data path). - Felix -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: add modparam 'hw_csum' to make HW checksum configurable
On 12/17/2015 02:57 PM, Felix Fietkau wrote: On 2015-12-17 23:01, Peter Oh wrote: On 12/16/2015 03:59 PM, Felix Fietkau wrote: On 2015-12-17 00:50, Peter Oh wrote: On 12/16/2015 01:54 PM, Felix Fietkau wrote: On 2015-12-16 22:19, Peter Oh wrote: On 12/16/2015 12:53 PM, Felix Fietkau wrote: On 2015-12-16 21:46, Peter Oh wrote: On 12/16/2015 12:35 PM, Felix Fietkau wrote: On 2015-12-16 21:29, Peter Oh wrote: On 12/16/2015 10:27 AM, Felix Fietkau wrote: On 2015-12-16 19:20, Peter Oh wrote: Some hardwares such as QCA988X and QCA99X0 doesn't have capability of checksum offload when frame formats are not suitable for it such as Mesh frame. Hence add a module parameter, hw_csum, to make checksum offload configurable during module registration time. Signed-off-by: Peter Oh How about instead of inventing yet another crappy module parameter, you call skb_checksum_help() in the driver in cases where the hardware is unable to offload the checksum calculation. That way the user has to worry about less driver specific hackery ;) That will be good option for hardware not supporting HW checksum, but I mind that using the function will add more workload per every packet on critical data path when HW supports checksum resulting in throughput down. I didn't mean calling it for every single frame in the data path. What I'm suggesting is calling it selectively only for mesh frames, or any other frames that the hardware cannot offload, and leaving the rest for the hardware to process. There should be no performance difference between disabling checksum offload and calling skb_checksum_help from the driver. To call it selectively for Mesh frame or interface, we need to add it on mac80211 layer such as ieee80211_build_hdr() since driver layer does not care the interface type in data path. No need to change mac80211 - it only touches the headers, and skb_checksum_help does not care about that. The skb has enough information for it to find the right range to calculate the checksum and the place to store it. If mentioned to use the function to mesh frame only without touching mac80211, then how do you suggest it to apply it only to mesh frame without interfere other data frames? Can you share your example? It's trivial - in ath10k_tx you do this: if (vif->type == NL80211_IFTYPE_MESH_POINT && skb->ip_summed == CHECKSUM_PARTIAL) skb_checksum_help(skb); Thank you Felix for the quick response. I agree on your user experience opinion, but what do you think when ath10k has a new chip supporting HW checksum for Mesh? Then you simply update the checks. What's the big deal? keep adding condition to such data path is not a good option. I also considered again about user experiences and reached to that this patch won't disturb user experience since the products will ship with proper module settings. for instance the parameter will be turned on if product support it other wise will be turned off as they shipped, so that users don't need to touch it. I think the point you were missing is the one that there is no such thing as a proper setting for this module parameter, since it doesn't really depend much on the hardware or the product, but on the wifi mode that you are using. In addition, for enterprise customers, they do care even a very small performance drop or enhancement especially when they are running BMT among vendors. So we need to avoid adding extra codes in data path in my opinion. The regular data tx path already checks ar->dev_flags to decide whether to use raw mode or not. This means that this part of the data structure is already cached. The vif type is also cached, since it's accessed in the same part of the function. Because of that, the impact of adding an extra check even for a hardware capability will be so low, that I'm pretty sure you will not be able to measure it. And even if it were measurable, it's probably quite easy to find a few places to optimize I find the tradeoff you are making very odd: For users that don't know about the module parameter (depending on the default value) it either just randomly doesn't work in mesh or always runs with degraded performance. All this to save adding a check that will be completely irrelevant for performance, since it won't result in any extra cache stalls (which are the typical bottleneck in the data path). Thank you for your comments and ideas. I'll spend more time to lead better solution based on you & Michal's feedback. - Felix Peter -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] bcma: switch GPIO portions to use GPIOLIB_IRQCHIP
I'm afraid it wasn't tested on BCM47XX (MIPS) :( On 14 August 2015 at 00:21, Hauke Mehrtens wrote: > @@ -218,9 +187,8 @@ int bcma_gpio_init(struct bcma_drv_cc *cc) > chip->set = bcma_gpio_set_value; > chip->direction_input = bcma_gpio_direction_input; > chip->direction_output = bcma_gpio_direction_output; > -#if IS_BUILTIN(CONFIG_BCM47XX) || IS_BUILTIN(CONFIG_ARCH_BCM_5301X) > - chip->to_irq= bcma_gpio_to_irq; > -#endif > + chip->owner = THIS_MODULE; > + chip->dev = bcma_bus_get_host_dev(bus); This assigns &bus->host_pdev->dev which is NULL. > @@ -248,13 +216,13 @@ int bcma_gpio_init(struct bcma_drv_cc *cc) > else > chip->base = -1; > > - err = bcma_gpio_irq_domain_init(cc); > + err = gpiochip_add(chip); > if (err) > return err; > > - err = gpiochip_add(chip); > + err = bcma_gpio_irq_init(cc); This results in: [0.157054] missing gpiochip .dev parent pointer (coming from gpiochip_irqchip_add) and [0.157287] bcma: bus0: Error registering GPIO driver: -22 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html