RE: [PATCH v7] Add new mac80211 driver mwlwifi.
> From: Julian Calaby [mailto:julian.cal...@gmail.com] Thursday, March 17, 2016
> 7:24 AM
>
> Hi David,
>
> On Thu, Nov 12, 2015 at 2:51 PM, David Lin wrote:
> > This patch provides the mwlwifi driver for Marvell 8863, 8864 and 8897
> > chipsets.
> > This driver was developed as part of the openwrt.org project to
> > support Linksys WRT1900AC and is maintained on
> https://github.com/kaloz/mwlwifi.
> >
> > The mwlwifi driver differs from existing mwifiex driver:
> > o mwlwifi is a "softmac driver" using the kernel? mac802.11 subsystem
> > to provide full AP/Wireless Bridge functionality (routers).
> > o mwifiex is a "fullmac driver" which provides a comprehensive set of
> > client functions (laptops/embedded devices) o only mwlwifi supports
> > Marvell AP chip 886X series
> >
> > NOTE: Users with Marvell 88W8897 chipsets currently should enable
> > (CONFIG=Y or M) either CONFIG_MWIFIEX or CONFIG_MWLWIFI, NOT
> BOTH.
> >
> > mwlwifi driver leveraged code from existing MWL8K driver in the
> > following areas:
> > - 802.11n setting for mac80211
> > - Functions needed to hook up to mac80211
> > - Interactions with mac80211 to establish BA streams
> > - Partial firmware APIs, some data fields
> > - Method to pass Rx packets to mac80211 except 11ac rates
> >
> > In addition, mwlwifi driver supports:
> > - future scalability and future development (refactored source code)
> > - Marvell 802.11ac chipsets, including combo BT devices
> > - 802.11ac related settings and functions
> > - concurrent AP+STA functionalities with single firmware per chip
> > - firmware APIs for the supported chipset
> > - communicating new mac80211 settings to firmware
> > - Different TX/RX datapath where applicable
> > - A-MSDU and A-MPDU
> > - mac80211-based MESH (work in progress)
> > - Refined the code to establish BA streams
> >
> > NOTE: MWLWIFI will be organized under new vendor specific
> > folder/marvell, as per request of the gate keeper and community.
> >
> > Signed-off-by: David Lin
>
> It's been quite a while since this was reviewed. Do you have a new version of
> the driver?
>
Yes we have some updates, and continue working on it. But we felt that
community does not seem to be very interested in this driver, so we have not
been updating the wireless org. Let us know if you are supportive and waiting
for it.
Thanks,
David
>
> Thanks,
>
> --
> Julian Calaby
>
> Email: julian.cal...@gmail.com
> Profile: http://www.google.com/profiles/julian.calaby/
N�r��yb�X��ǧv�^�){.n�+{��*ޕ�,�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj"��!�i
RE: [PATCH v7] Add new mac80211 driver mwlwifi.
> From: Julian Calaby [mailto:julian.cal...@gmail.com] Thursday, March 17, 2016 > 9:32 AM> > Hi David, > > On Thu, Mar 17, 2016 at 12:23 PM, David Lin wrote: > >> From: Julian Calaby [mailto:julian.cal...@gmail.com] Thursday, March > >> 17, 2016 7:24 AM > >> > >> Hi David, > >> > >> On Thu, Nov 12, 2015 at 2:51 PM, David Lin wrote: > >> > This patch provides the mwlwifi driver for Marvell 8863, 8864 and > >> > 8897 chipsets. > >> > This driver was developed as part of the openwrt.org project to > >> > support Linksys WRT1900AC and is maintained on > >> https://github.com/kaloz/mwlwifi. > >> > > >> > The mwlwifi driver differs from existing mwifiex driver: > >> > o mwlwifi is a "softmac driver" using the kernel? mac802.11 > >> > subsystem to provide full AP/Wireless Bridge functionality (routers). > >> > o mwifiex is a "fullmac driver" which provides a comprehensive set > >> > of client functions (laptops/embedded devices) o only mwlwifi > >> > supports Marvell AP chip 886X series > >> > > >> > NOTE: Users with Marvell 88W8897 chipsets currently should enable > >> > (CONFIG=Y or M) either CONFIG_MWIFIEX or CONFIG_MWLWIFI, NOT > >> BOTH. > >> > > >> > mwlwifi driver leveraged code from existing MWL8K driver in the > >> > following areas: > >> > - 802.11n setting for mac80211 > >> > - Functions needed to hook up to mac80211 > >> > - Interactions with mac80211 to establish BA streams > >> > - Partial firmware APIs, some data fields > >> > - Method to pass Rx packets to mac80211 except 11ac rates > >> > > >> > In addition, mwlwifi driver supports: > >> > - future scalability and future development (refactored source > >> > code) > >> > - Marvell 802.11ac chipsets, including combo BT devices > >> > - 802.11ac related settings and functions > >> > - concurrent AP+STA functionalities with single firmware per chip > >> > - firmware APIs for the supported chipset > >> > - communicating new mac80211 settings to firmware > >> > - Different TX/RX datapath where applicable > >> > - A-MSDU and A-MPDU > >> > - mac80211-based MESH (work in progress) > >> > - Refined the code to establish BA streams > >> > > >> > NOTE: MWLWIFI will be organized under new vendor specific > >> > folder/marvell, as per request of the gate keeper and community. > >> > > >> > Signed-off-by: David Lin > >> > >> It's been quite a while since this was reviewed. Do you have a new > >> version of the driver? > >> > > > > Yes we have some updates, and continue working on it. But we felt that > community does not seem to be very interested in this driver, so we have not > been updating the wireless org. Let us know if you are supportive and waiting > for it. > > My understanding was that we'd very much like the driver to be sent upstream. > From my perspective, it looked like you'd lost interest, not us. > We felt some resistance from various people, and get discouraged. Though we continue to work on it internally, we were just keeping further submission at the back of our mind. We do keep the feedback from community and will work on compliancy where applicable and possible. Next, we will prepare a patch8 for submission later. > > Thanks, > > -- > Julian Calaby > > Email: julian.cal...@gmail.com > Profile: http://www.google.com/profiles/julian.calaby/
RE: [PATCH] Add FW binary used by mwlwifi driver.
> Ben Hutchings wrote:
>
> On Sat, 2015-08-22 at 03:54 +0000, David Lin wrote:
> > Signed-off-by: David Lin
> > ---
> > WHENCE | 12
> > mwlwifi/88W8864.bin | Bin 0 -> 116356 bytes
> > mwlwifi/88W8897.bin | Bin 0 -> 489084 bytes
> > 3 files changed, 12 insertions(+)
> > mode change 100644 => 100755 WHENCE
> > create mode 100755 mwlwifi/88W8864.bin
> > create mode 100755 mwlwifi/88W8897.bin
> [...]
>
> Applied. In future, please send requests to the alias
> linux-firmw...@kernel.org.
>
Thanks. I will.
>
> Ben.
>
> --
> Ben Hutchings
> A free society is one where it is safe to be unpopular. - Adlai Stevenson
David
N�r��yb�X��ǧv�^�){.n�+{��*ޕ�,�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj"��!�i
RE: [PATCH v7] Add new mac80211 driver mwlwifi.
> On November 20, 2015 7:22 PM, Johannes Berg wrote:
>
> On Thu, 2015-11-12 at 03:51 +0000, David Lin wrote:
> >
> > +static int print_mac_addr(char *p, u8 *mac_addr) {
> > + int i;
> > + char *str = p;
> > +
> > + str += sprintf(str, "mac address: %02x", mac_addr[0]);
> > + for (i = 1; i < ETH_ALEN; i++)
> > + str += sprintf(str, ":%02x", mac_addr[i]);
> > + str += sprintf(str, "\n");
> > +
> > + return (str - p);
> > +}
>
> You can use %pM here (and perhaps get rid of the function then)
>
I will modify it.
> > +void mwl_debugfs_remove(struct ieee80211_hw *hw) {
> > + struct mwl_priv *priv = hw->priv;
> > +
> > + debugfs_remove(priv->debugfs_phy);
>
> doesn't that have to be _recursive()?
>
> > +#define MWL_DRV_NAME KBUILD_MODNAME
> > +#define MWL_DRV_VERSION "10.3.0.14"
>
> versions like that are generally fairly useless since you don't update them
> for
> every commit ...
>
We still need version information for formal release.
> > +struct mwl_tx_desc {
> > + u8 data_rate;
> > + u8 tx_priority;
> > + __le16 qos_ctrl;
> [...]
>
> I still wouldn't mix device/firmware API with driver-use structures in the
> same
> file, it gets confusing. It's your driver though, so if you really want to
> confuse
> yourselves ... assuming you're going to maintain it :)
>
These fields are firmware API. Yes, we will maintain it.
> > +struct mwl_sta {
> > + struct list_head list;
> > + bool is_mesh_node;
> > + bool is_ampdu_allowed;
> > + struct mwl_tx_info tx_stats[MWL_MAX_TID];
> > + bool is_amsdu_allowed;
> > + spinlock_t amsdu_lock; /* for amsdu
> > aggregation */
> > + struct mwl_amsdu_ctrl amsdu_ctrl;
> > + u16 iv16;
> > + u32 iv32;
> > +};
>
> I still don't see how this iv stuff in the *station* can possibly be right. I
> think I
> also pointed out earlier that you can just let
> mac80211 assign the PN/IV.
>
I will try to modify the code to remove them.
> > +static int mwl_fwcmd_wait_complete(struct mwl_priv *priv, unsigned
> > short cmd)
> > +{
> >
> [...]
> > + mdelay(3);
> > +
> > + return 0;
> > +}
> > +
> > +static int mwl_fwcmd_exec_cmd(struct mwl_priv *priv, unsigned short
> > cmd)
> > +{
> [...]
> > + if (mwl_fwcmd_wait_complete(priv, 0x8000 | cmd)) {
> [...]
> > +}
> > +
> > +static int mwl_fwcmd_802_11_radio_control(struct mwl_priv *priv,
> > + bool enable, bool force)
> > +{
> [...]
> > + spin_lock_bh(&priv->fwcmd_lock);
> [...]
> > + if (mwl_fwcmd_exec_cmd(priv,
> > HOSTCMD_CMD_802_11_RADIO_CONTROL)) {
> > + spin_unlock_bh(&priv->fwcmd_lock);
>
> This seems like a terrible idea. Spinning for 3 milliseconds (and more) while
> blocking local soft-irqs is really bad for general latency on the system. I
> don't
> like it at all.
>
> And there doesn't really seem to be much reason for it either as far as I can
> tell - almost all places can (as I pointed out before) live with this being a
> mutex, you just need special handling in very few places that really do want
> to
> send a command while not being able to sleep.
>
I will try to use mutex in next patch soon.
> > +++ b/drivers/net/wireless/marvell/mwlwifi/hostcmd.h
>
> This looks like you do have a file for commands - maybe move that TX thing
> above here?
>
It is better to keep current file structure (easier for our internal
maintenance syncing other files)
> > +const struct ieee80211_ops mwl_mac80211_ops = {
> > + .tx = mwl_mac80211_tx,
> > + .start = mwl_mac80211_start,
> > + .stop = mwl_mac80211_stop,
> > + .add_interface = mwl_mac80211_add_interface,
> > + .remove_interface = mwl_mac80211_remove_interface,
> > + .config = mwl_mac80211_config,
> > + .bss_info_changed = mwl_mac80211_bss_info_changed,
> > + .configure_filter = mwl_mac80211_configure_filter,
> > + .set_key= mwl_mac80211_set_key,
> > + .set_rts_threshold = mwl_mac80211_set_rts_threshold,
> > + .sta_add= mwl_mac80211_sta_add,
> > + .sta_remove = mwl_mac80211_sta_remove,
> > + .conf_tx= mwl_mac80211_conf_tx,
> > + .get_stats = mwl_mac80211_get_stats,
> > + .get_survey = mwl_mac80211_get_survey,
&
RE: [PATCH v7] Add new mac80211 driver mwlwifi.
> On November 13, 2015 3:48 AM, Jes Sorensen wrote: > > David Lin writes: > > This patch provides the mwlwifi driver for Marvell 8863, 8864 and 8897 > > chipsets. > > This driver was developed as part of the openwrt.org project to > > support Linksys WRT1900AC and is maintained on > https://github.com/kaloz/mwlwifi. > > > > The mwlwifi driver differs from existing mwifiex driver: > > o mwlwifi is a "softmac driver" using the kernel? mac802.11 subsystem > > to provide full AP/Wireless Bridge functionality (routers). > > o mwifiex is a "fullmac driver" which provides a comprehensive set of > > client functions (laptops/embedded devices) o only mwlwifi supports > > Marvell AP chip 886X series > > > > NOTE: Users with Marvell 88W8897 chipsets currently should enable > > (CONFIG=Y or M) either CONFIG_MWIFIEX or CONFIG_MWLWIFI, NOT > BOTH. > > I didn't get very far reading through this - but there was a few bits that > stood > out like a sore thumb. > > > mwlwifi driver leveraged code from existing MWL8K driver in the > > following areas: > > - 802.11n setting for mac80211 > > - Functions needed to hook up to mac80211 > > - Interactions with mac80211 to establish BA streams > > - Partial firmware APIs, some data fields > > - Method to pass Rx packets to mac80211 except 11ac rates > > > > In addition, mwlwifi driver supports: > > - future scalability and future development (refactored source code) > > - Marvell 802.11ac chipsets, including combo BT devices > > - 802.11ac related settings and functions > > - concurrent AP+STA functionalities with single firmware per chip > > - firmware APIs for the supported chipset > > - communicating new mac80211 settings to firmware > > - Different TX/RX datapath where applicable > > - A-MSDU and A-MPDU > > - mac80211-based MESH (work in progress) > > - Refined the code to establish BA streams > > > > NOTE: MWLWIFI will be organized under new vendor specific > > folder/marvell, as per request of the gate keeper and community. > > > > Signed-off-by: David Lin > > --- > > > > diff --git a/MAINTAINERS b/MAINTAINERS index 27b27c0..7c32f0a 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -6629,6 +6629,12 @@ L: linux-wireless@vger.kernel.org > > S: Odd Fixes > > F: drivers/net/wireless/mwl8k.c > > > > +MARVELL MWLWIFI WIRELESS DRIVER > > +M: David Lin > > +L: linux-wireless@vger.kernel.org > > +S: Maintained > > +F: drivers/net/wireless/marvell/mwlwifi > > + > > MARVELL SOC MMC/SD/SDIO CONTROLLER DRIVER > > M: Nicolas Pitre > > S: Odd Fixes > > diff --git a/drivers/net/wireless/Kconfig > > b/drivers/net/wireless/Kconfig index f9f9422..d599b35 100644 > > --- a/drivers/net/wireless/Kconfig > > +++ b/drivers/net/wireless/Kconfig > > @@ -285,5 +285,6 @@ source "drivers/net/wireless/zd1211rw/Kconfig" > > source "drivers/net/wireless/mwifiex/Kconfig" > > source "drivers/net/wireless/cw1200/Kconfig" > > source "drivers/net/wireless/rsi/Kconfig" > > +source "drivers/net/wireless/marvell/Kconfig" > > > > endif # WLAN > > diff --git a/drivers/net/wireless/Makefile > > b/drivers/net/wireless/Makefile index 740fdd3..71504a7 100644 > > --- a/drivers/net/wireless/Makefile > > +++ b/drivers/net/wireless/Makefile > > @@ -60,3 +60,5 @@ obj-$(CONFIG_BRCMSMAC)+= brcm80211/ > > > > obj-$(CONFIG_CW1200) += cw1200/ > > obj-$(CONFIG_RSI_91X) += rsi/ > > + > > +obj-$(CONFIG_WL_MARVELL) += marvell/ > > diff --git a/drivers/net/wireless/marvell/Kconfig > > b/drivers/net/wireless/marvell/Kconfig > > new file mode 100644 > > index 000..d73e642 > > --- /dev/null > > +++ b/drivers/net/wireless/marvell/Kconfig > > @@ -0,0 +1,8 @@ > > +menuconfig WL_MARVELL > > + bool "Marvell Wireless LAN support" > > + ---help--- > > + Enable community drivers for Marvell Wi-Fi devices. > > + > > +if WL_MARVELL > > +source "drivers/net/wireless/marvell/mwlwifi/Kconfig" > > +endif # WL_MARVELL > > diff --git a/drivers/net/wireless/marvell/Makefile > > b/drivers/net/wireless/marvell/Makefile > > new file mode 100644 > > index 000..70d1b3f > > --- /dev/null > > +++ b/drivers/net/wireless/marvell/Makefile > > @@ -0,0 +1 @@ > > +obj-$(CONFIG_MWLWIFI) += mwlwifi/ > > diff --git a/drivers/net/wireless/marvell/mwlwifi/Kconfig > > b/drivers/net
RE: [PATCH v7] Add new mac80211 driver mwlwifi.
> On November 26, 2015 5:40 PM, Johannes Berg wrote: > > On Thu, 2015-11-26 at 08:27 +0000, David Lin wrote: > > > > Since you do this in .probe, you should consider loading the > > > firmware asynchronously. > > > > > > > I hope I can postpone this modification later. > > I don't feel strongly about this one - otoh it's not really complicated. > Thanks. > > > > +#ifdef CONFIG_SUPPORT_MFG > > > > > > This Kconfig variable doesn't exist. > > > > > > > The compile variable is used privately by Marvell and our customers in > > production line. > > Yeah, still. Make it a proper Kconfig variable, defaulting to off and hidden > under something, or remove it. It's extremely misleading to have something > called CONFIG_* when it's not a Kconfig variable. > I will change this compile variable from "CONFIG_SUPPORT_MFG" to "SUPPORT_MFG". > > Mac80211 does not support mesh AMSDU now, we implement this function > > in mwlwifi driver for the time being. Except for mesh AMSDU, we still > > leverage mac80211 to handle data AMSDU. > > You're allowed to modify mac80211. > > > > What's going on here? Why are you modifying the action frames on the > > > fly??! > > > > > > > Due to mwlwifi supports Tx/Rx AMSDU, these frames are modified to > > inform client that we support AMSDU. > > > > Ditto - you're allowed to modify mac80211. We actually have a patch like this > already: > https://git.kernel.org/cgit/linux/kernel/git/jberg/mac80211- > next.git/commit/?id=99e7ca44bb910f0cbfda5d9008e8517df0ebc939 > Once if this patch is ready, I will modify mwlwifi to use it. 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 v7] Add new mac80211 driver mwlwifi.
> Kan Yan writes:
>
> David Lin writes:
>
> > +static inline struct sk_buff *mwl_tx_do_amsdu(struct mwl_priv *priv,
> > + int desc_num,
> > ...
> > + amsdu_pkts = (struct sk_buff_head *)
> > + kmalloc(sizeof(*amsdu_pkts), GFP_KERNEL);
> > + if (!amsdu_pkts) {
>
> Should GFP_ATOMIC be used here instead of GFP_KERNEL? This function could
> be called in interrupt context.
>
Thanks. I will fix 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
--
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 v7] Add new mac80211 driver mwlwifi.
> Emmanuel Grumbach [mailto:egrumb...@gmail.com] wrote:
>
> Hi,
>
> I was interested in the checking the A-MSDU implementation only, so I
> reviewed only that.
>
> I am not sure I really followed your implementation though.
> In iwlwifi I chose the LSO mechanism to implement A-MSDU. I understand that
> for an AP it makes less sense since you don't really have locally generated
> traffic and LSO won't work well with packets forwarded by the IP layer.
> So you seem to have added a timeout mechanism inside the driver that
> accumulates the packet for a certain period of time and send them all when
> the max number of packets has been reached?
> But I didn't see any timer there, so that if you start an A-MSDU and then
> suddenly stop receiving packets for transmission, the packet already in the
> buffer will stay stale?
>
The flush mechanism is hooked on queue empty interrupt.
> A few more comments below.
>
> On Thu, Nov 12, 2015 at 5:51 AM, David Lin wrote:
> > This patch provides the mwlwifi driver for Marvell 8863, 8864 and 8897
> > chipsets.
> > This driver was developed as part of the openwrt.org project to
> > support Linksys WRT1900AC and is maintained on
> https://github.com/kaloz/mwlwifi.
> >
> > The mwlwifi driver differs from existing mwifiex driver:
> > o mwlwifi is a "softmac driver" using the kernel? mac802.11 subsystem
> > to provide full AP/Wireless Bridge functionality (routers).
> > o mwifiex is a "fullmac driver" which provides a comprehensive set of
> > client functions (laptops/embedded devices) o only mwlwifi supports
> > Marvell AP chip 886X series
> >
>
> [snip]
>
> > +
> > +static int mwl_mac80211_ampdu_action(struct ieee80211_hw *hw,
> > +struct ieee80211_vif *vif,
> > +enum
> ieee80211_ampdu_mlme_action action,
> > +struct ieee80211_sta *sta,
> > +u16 tid, u16 *ssn, u8 buf_size,
> > +bool amsdu) {
> > + int rc = 0;
> > + struct mwl_priv *priv = hw->priv;
> > + struct mwl_ampdu_stream *stream;
> > + u8 *addr = sta->addr, idx;
> > + struct mwl_sta *sta_info;
> > +
> > + sta_info = mwl_dev_get_sta(sta);
> > +
> > + spin_lock_bh(&priv->stream_lock);
> > +
> > + stream = mwl_fwcmd_lookup_stream(hw, addr, tid);
> > +
> > + switch (action) {
> > + case IEEE80211_AMPDU_RX_START:
> > + case IEEE80211_AMPDU_RX_STOP:
> > + break;
> > + case IEEE80211_AMPDU_TX_START:
> > + if (!sta_info->is_ampdu_allowed) {
> > + wiphy_warn(hw->wiphy, "ampdu not
> allowed\n");
> > + rc = -EPERM;
> > + break;
> > + }
> > +
> > + if (!stream) {
> > + stream = mwl_fwcmd_add_stream(hw, sta,
> tid);
> > + if (!stream) {
> > + wiphy_warn(hw->wiphy, "no stream
> found\n");
> > + rc = -EPERM;
> > + break;
> > + }
> > + }
> > +
> > + spin_unlock_bh(&priv->stream_lock);
> > + rc = mwl_fwcmd_check_ba(hw, stream, vif);
> > + spin_lock_bh(&priv->stream_lock);
> > + if (rc) {
> > + mwl_fwcmd_remove_stream(hw, stream);
> > + wiphy_err(hw->wiphy,
> > + "ampdu start error code: %d\n",
> rc);
> > + rc = -EPERM;
> > + break;
> > + }
> > + stream->state = AMPDU_STREAM_IN_PROGRESS;
> > + *ssn = 0;
> > + ieee80211_start_tx_ba_cb_irqsafe(vif, addr, tid);
>
> This is why you have the race you mention below. What you should really be
> doing is to wait until the all the packets for that RA / TID are sent from
> all the
> HW Tx queues (only one realistically), and only then call the _cb. This will
> allow you to send the ADDBA req on the VO queue as it should be according to
> spec.
>
> [snip]
>
> > +
> > +void mwl_rx_recv(unsigned long data)
> > +{
>
> [snip]
>
> > +
> > + if
> (unlikely(i
RE: [PATCH v7] Add new mac80211 driver mwlwifi.
> Kalle Valo [mailto:kv...@codeaurora.org] wrote: > > David Lin writes: > > > On November 26, 2015 5:40 PM, Johannes Berg wrote: > >> On Thu, 2015-11-26 at 08:27 +, David Lin wrote: > >> > >> > > > +#ifdef CONFIG_SUPPORT_MFG > >> > > > >> > > This Kconfig variable doesn't exist. > >> > > > >> > > >> > The compile variable is used privately by Marvell and our customers > >> > in production line. > >> > >> Yeah, still. Make it a proper Kconfig variable, defaulting to off and > >> hidden under something, or remove it. It's extremely misleading to > >> have something called CONFIG_* when it's not a Kconfig variable. > >> > > > > I will change this compile variable from "CONFIG_SUPPORT_MFG" to > > "SUPPORT_MFG". > > Then it's still dead code which won't ever get compiled in upstream. > Please follow what Johannes suggested. > The code will be removed. > -- > 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: [PATCH v7] Add new mac80211 driver mwlwifi.
> Kalle Valo [mailto:kv...@codeaurora.org] wrote: > > David Lin writes: > > > This patch provides the mwlwifi driver for Marvell 8863, 8864 and 8897 > > chipsets. > > This driver was developed as part of the openwrt.org project to > > support Linksys WRT1900AC and is maintained on > https://github.com/kaloz/mwlwifi. > > > > The mwlwifi driver differs from existing mwifiex driver: > > o mwlwifi is a "softmac driver" using the kernel? mac802.11 subsystem > > to provide full AP/Wireless Bridge functionality (routers). > > o mwifiex is a "fullmac driver" which provides a comprehensive set of > > client functions (laptops/embedded devices) o only mwlwifi supports > > Marvell AP chip 886X series > > > > NOTE: Users with Marvell 88W8897 chipsets currently should enable > > (CONFIG=Y or M) either CONFIG_MWIFIEX or CONFIG_MWLWIFI, NOT > BOTH. > > > > mwlwifi driver leveraged code from existing MWL8K driver in the > > following areas: > > - 802.11n setting for mac80211 > > - Functions needed to hook up to mac80211 > > - Interactions with mac80211 to establish BA streams > > - Partial firmware APIs, some data fields > > - Method to pass Rx packets to mac80211 except 11ac rates > > > > In addition, mwlwifi driver supports: > > - future scalability and future development (refactored source code) > > - Marvell 802.11ac chipsets, including combo BT devices > > - 802.11ac related settings and functions > > - concurrent AP+STA functionalities with single firmware per chip > > - firmware APIs for the supported chipset > > - communicating new mac80211 settings to firmware > > - Different TX/RX datapath where applicable > > - A-MSDU and A-MPDU > > - mac80211-based MESH (work in progress) > > - Refined the code to establish BA streams > > > > NOTE: MWLWIFI will be organized under new vendor specific > > folder/marvell, as per request of the gate keeper and community. > > > > Signed-off-by: David Lin > > This seems to use base64 encoding, how did you submit this? 'git send-email' > tool is strongly preferred. > > Content-Transfer-Encoding: base64 > > I applied this to pending branch and saw few easy conflicts with Kconfig files > and Makefiles, I guess you submitted this patch before I added the vendor > directories. Let's see what kbuild finds but I already saw two > warnings: > > drivers/net/wireless/marvell/mwlwifi/main.c:160:20: warning: incorrect type > in argument 1 (different address spaces) > drivers/net/wireless/marvell/mwlwifi/main.c:160:20:expected void const > *ptr > drivers/net/wireless/marvell/mwlwifi/main.c:160:20:got void [noderef] > *[assigned] addr > drivers/net/wireless/marvell/mwlwifi/main.c:171:20: warning: incorrect type > in argument 1 (different address spaces) > drivers/net/wireless/marvell/mwlwifi/main.c:171:20:expected void const > *ptr > drivers/net/wireless/marvell/mwlwifi/main.c:171:20:got void [noderef] > *[assigned] addr > I will fix it. > -- > Kalle Valo
Re: [PATCH v9] Add new mac80211 driver mwlwifi.
> From: steve.deros...@gmail.com Wrote:
> Hi David,
>
> First off, I wanted to say thank-you for your work and effort in
> trying to get mwlwifi upstream. My comments are in-line with my
> general notes afterwards.
>
> On Tue, Dec 20, 2016 at 8:11 PM, David Lin wrote:
>
> > diff --git a/drivers/net/wireless/marvell/mwlwifi/debugfs.h
> > b/drivers/net/wireless/marvell/mwlwifi/debugfs.h
> > new file mode 100644
> > +#ifndef _MWL_DEBUGFS_H_
> > +#define _MWL_DEBUGFS_H_
> > +
> > +void mwl_debugfs_init(struct ieee80211_hw *hw); void
> > +mwl_debugfs_remove(struct ieee80211_hw *hw);
> > +
>
> You should guard these so they define to empty functions if
> CONFIG_DEBUG_FS isn't enabled so they can be used by a caller without
> explicit guards.
>
I will modify it.
>
> > diff --git a/drivers/net/wireless/marvell/mwlwifi/dev.h
> > b/drivers/net/wireless/marvell/mwlwifi/dev.h
> > new file mode 100644
> > index 000..c7b10ac
> > --- /dev/null
> > +++ b/drivers/net/wireless/marvell/mwlwifi/dev.h
> > +#ifdef CONFIG_DEBUG_FS
> > +#define MAC_REG_ADDR_PCI(offset) ((priv->iobase1 + 0xA000) +
> (offset))
> > +
> > +#define MWL_ACCESS_MAC1
> > +#define MWL_ACCESS_RF 2
> > +#define MWL_ACCESS_BBP3
> > +#define MWL_ACCESS_CAU4
> > +#define MWL_ACCESS_ADDR0 5
> > +#define MWL_ACCESS_ADDR1 6
> > +#define MWL_ACCESS_ADDR 7
> > +#endif
>
> OK, I get that you're only using the above in the debugfs functions,
> but as for generic register access functions, these would be valid no
> matter if CONFIG_DEBUG_FS is defined. Having the guard here just seems
> to complicate things.
>
I will remove it.
>
> > +
> > +struct mwl_priv {
> > + struct ieee80211_hw *hw;
> > + struct firmware *fw_ucode;
> > + bool fw_device_pwrtbl;
> > + bool forbidden_setting;
> > +
> > + struct thermal_cooling_device *cdev;
> > + u32 throttle_state;
> > + u32 quiet_period;
> > + int temperature;
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > + struct dentry *debugfs_phy;
> > + u32 reg_type;
> > + u32 reg_offset;
> > + u32 reg_value;
> > + int tx_desc_num;
> > +#endif
>
> We're saving a few bytes here at the cost of some complexity. I could
> go either way, but I hate when priv structures mutate.
>
I will remove the compile control.
>
> > +/* Defined in mac80211.c. */
> > +extern const struct ieee80211_ops mwl_mac80211_ops;
>
> Does this need to be in dev.h?
>
>
It can be moved to main.c.
>
> > diff --git a/drivers/net/wireless/marvell/mwlwifi/fwcmd.c
> > b/drivers/net/wireless/marvell/mwlwifi/fwcmd.c
> > new file mode 100644
> > index 000..9c3ccf9
> > --- /dev/null
> > +++ b/drivers/net/wireless/marvell/mwlwifi/fwcmd.c
> > @@ -0,0 +1,2837 @@
> > +/*
> > + * Copyright (C) 2006-2016, Marvell International Ltd.
> > + *
> > + * This software file (the "File") is distributed by Marvell
> > +International
> > + * Ltd. under the terms of the GNU General Public License Version
> > +2, June 1991
> > + * (the "License"). You may use, redistribute and/or modify this
> > +
> > +#define MAX_WAIT_FW_COMPLETE_ITERATIONS 2000
> > +#define MAX_WAIT_GET_HW_SPECS_ITERATONS 3
> > +
> > +struct cmd_header {
> > + __le16 command;
> > + __le16 len;
> > +} __packed;
> > +
> > +static bool mwl_fwcmd_chk_adapter(struct mwl_priv *priv) {
> > + u32 regval;
> > +
> > + regval = readl(priv->iobase1 + MACREG_REG_INT_CODE);
>
> Couldn't the above be one line?
>
Yes. I will modify it.
>
> > +
> > +static int mwl_fwcmd_get_tx_powers(struct mwl_priv *priv, u16
> > +*powlist,
> u16 ch,
> > + u16 band, u16 width, u16 sub_ch) { struct
> > +hostcmd_cmd_802_11_tx_power *pcmd; int i;
> > +
> > + pcmd = (struct hostcmd_cmd_802_11_tx_power *)&priv->pcmd_buf[0];
>
> Often happens, so I probably won't catch them all, but this could
> likewise be done in one line. Then again... line length issues, so 50/50 on
> that.
>
>
I will modify it.
> > +
> > +static u8 mwl_fwcmd_get_80m_pri_chnl(u8 channel) {
> > + u8 act_primary = ACT_PRIMARY_CHAN_0;
> > +
> > + switch (channel) {
> > + case 36:
> > + act_primary = ACT_PRIMARY_CHAN_0;
> > + break;
> > + case 40:
> > + act_primary = ACT_PRIMARY_CHAN_1;
> >
RE: [EXT] Re: [PATCH v9] Add new mac80211 driver mwlwifi.
> From: steve.deros...@gmail.com Wrote:
> Hi David,
>
> First off, I wanted to say thank-you for your work and effort in trying to get
> mwlwifi upstream. My comments are in-line with my general notes
> afterwards.
>
> On Tue, Dec 20, 2016 at 8:11 PM, David Lin wrote:
>
> > diff --git a/drivers/net/wireless/marvell/mwlwifi/debugfs.h
> > b/drivers/net/wireless/marvell/mwlwifi/debugfs.h
> > new file mode 100644
> > index 000..b4c3eb3
> > +/* Description: This file defines debug fs related functions. */
> > +
> > +#ifndef _MWL_DEBUGFS_H_
> > +#define _MWL_DEBUGFS_H_
> > +
> > +void mwl_debugfs_init(struct ieee80211_hw *hw); void
> > +mwl_debugfs_remove(struct ieee80211_hw *hw);
> > +
>
> You should guard these so they define to empty functions if
> CONFIG_DEBUG_FS isn't enabled so they can be used by a caller without
> explicit guards.
>
I will modify it.
>
> > diff --git a/drivers/net/wireless/marvell/mwlwifi/dev.h
> > b/drivers/net/wireless/marvell/mwlwifi/dev.h
> > new file mode 100644
> > index 000..c7b10ac
> > --- /dev/null
> > +++ b/drivers/net/wireless/marvell/mwlwifi/dev.h
> ...
> > +struct mwl_ampdu_stream {
> > + struct ieee80211_sta *sta;
> > + u8 tid;
> > + u8 state;
> > + u8 idx;
> > +};
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > +#define MAC_REG_ADDR_PCI(offset) ((priv->iobase1 + 0xA000) +
> (offset))
> > +
> > +#define MWL_ACCESS_MAC1
> > +#define MWL_ACCESS_RF 2
> > +#define MWL_ACCESS_BBP3
> > +#define MWL_ACCESS_CAU4
> > +#define MWL_ACCESS_ADDR0 5
> > +#define MWL_ACCESS_ADDR1 6
> > +#define MWL_ACCESS_ADDR 7
> > +#endif
>
> OK, I get that you're only using the above in the debugfs functions, but as
> for
> generic register access functions, these would be valid no matter if
> CONFIG_DEBUG_FS is defined. Having the guard here just seems to
> complicate things.
>
I will remove it.
>
> > +
> > +struct mwl_priv {
> > + struct ieee80211_hw *hw;
> > + struct firmware *fw_ucode;
> > + bool fw_device_pwrtbl;
> > + bool forbidden_setting;
> > +
> > + struct thermal_cooling_device *cdev;
> > + u32 throttle_state;
> > + u32 quiet_period;
> > + int temperature;
> > +
> > +#ifdef CONFIG_DEBUG_FS
> > + struct dentry *debugfs_phy;
> > + u32 reg_type;
> > + u32 reg_offset;
> > + u32 reg_value;
> > + int tx_desc_num;
> > +#endif
>
> We're saving a few bytes here at the cost of some complexity. I could go
> either
> way, but I hate when priv structures mutate.
>
I will remove the compile control.
>
> > +/* Defined in mac80211.c. */
> > +extern const struct ieee80211_ops mwl_mac80211_ops;
>
> Does this need to be in dev.h?
>
>
It can be moved to main.c.
>
> > diff --git a/drivers/net/wireless/marvell/mwlwifi/fwcmd.c
> > b/drivers/net/wireless/marvell/mwlwifi/fwcmd.c
> > new file mode 100644
> > index 000..9c3ccf9
> > --- /dev/null
> > +++ b/drivers/net/wireless/marvell/mwlwifi/fwcmd.c
> > @@ -0,0 +1,2837 @@
> > +/*
> > + * Copyright (C) 2006-2016, Marvell International Ltd.
> > + *
> > + * This software file (the "File") is distributed by Marvell
> > +International
> > + * Ltd. under the terms of the GNU General Public License Version 2,
> > +June 1991
> > + * (the "License"). You may use, redistribute and/or modify this
> > +
> > +#define MAX_WAIT_FW_COMPLETE_ITERATIONS 2000
> > +#define MAX_WAIT_GET_HW_SPECS_ITERATONS 3
> > +
> > +struct cmd_header {
> > + __le16 command;
> > + __le16 len;
> > +} __packed;
> > +
> > +static bool mwl_fwcmd_chk_adapter(struct mwl_priv *priv) {
> > + u32 regval;
> > +
> > + regval = readl(priv->iobase1 + MACREG_REG_INT_CODE);
>
> Couldn't the above be one line?
>
Yes. I will modify it.
>
> > +
> > +static int mwl_fwcmd_get_tx_powers(struct mwl_priv *priv, u16 *powlist,
> u16 ch,
> > + u16 band, u16 width, u16 sub_ch)
> > +{
> > + struct hostcmd_cmd_802_11_tx_power *pcmd; int i;
> > +
> > + pcmd = (struct hostcmd_cmd_802_11_tx_power *)&priv->pcmd_buf[0];
>
> Often happens, so I probably won't catch them all, but this could likewise be
> done in one line. Then again... line length issues, so 50/50 on that.
>
>
I will modify it.
> > +
> > +static u8 m
Re: [PATCH v9] Add new mac80211 driver mwlwifi.
> Rafał Miłecki [mailto:zaj...@gmail.com] wrote:
> On 7 February 2017 at 20:12, Steve deRosier wrote:
> >> + /* look for all matching property names */
> >> + for_each_property_of_node(priv->dt_node, prop) { if
> >> + (strcmp(prop->name, "marvell,2ghz") == 0)
> >> + priv->disable_2g = true;
> >> + if (strcmp(prop->name, "marvell,5ghz") == 0)
> >> + priv->disable_5g = true;
> >> + if (strcmp(prop->name, "marvell,chainmask") == 0) { prop_value =
> >> + be32_to_cpu(*((__be32 *)prop->value)); if (prop_value == 2)
> >> + priv->antenna_tx = ANTENNA_TX_2;
> >> +
> >> + prop_value = be32_to_cpu(*((__be32 *) (prop->value + 4))); if
> >> + (prop_value == 2)
> >> + priv->antenna_rx = ANTENNA_RX_2;
> >> + }
> >> + }
> >> +
> >> + priv->pwr_node = of_find_node_by_name(priv->dt_node,
> >> + "marvell,powertable");
> >> +#endif
> >> +}
> >
> > AFAICT, there's no documentation for these DT bindings. The mwlwifi
> > node and the marvell,powertable both need full documentation in
> > Documentation/devicetree/bindings/... .
> >
> > Frankly I have a feeling I'm going to need these DT nodes and I'd like
> > to not have to guess-and-check based on the code.
>
> Please use ieee80211-freq-limit:
> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=b3
> 30b25eaabda00d74e47566d9200907da381896
>
> Most likely with wiphy_read_of_freq_limits helper:
> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=e6
> 91ac2f75b69bee743f0370d79454ba4429b175
I already replied meaning of these parameters:
is used to disable 2g band.
is used to disable 5g band.
is used to specify antenna number (if default number is
suitable for you, there is no need to use this parameter).
should not be used for chip with device power table. In
fact, this parameter should not be used any more.
Thanks,
David
RE: [PATCH v9] Add new mac80211 driver mwlwifi.
steve.deros...@gmail.com [mailto:steve.deros...@gmail.com] wrote:
> On Tue, Feb 7, 2017 at 10:15 PM, David Lin wrote:
> >> Rafał Miłecki [mailto:zaj...@gmail.com] wrote:
> >> On 7 February 2017 at 20:12, Steve deRosier wrote:
> >> >> + /* look for all matching property names */
> >> >> + for_each_property_of_node(priv->dt_node, prop) { if
> >> >> + (strcmp(prop->name, "marvell,2ghz") == 0)
> >> >> + priv->disable_2g = true;
> >> >> + if (strcmp(prop->name, "marvell,5ghz") == 0)
> >> >> + priv->disable_5g = true;
> >> >> + if (strcmp(prop->name, "marvell,chainmask") == 0) { prop_value =
> >> >> + be32_to_cpu(*((__be32 *)prop->value)); if (prop_value == 2)
> >> >> + priv->antenna_tx = ANTENNA_TX_2;
> >> >> +
> >> >> + prop_value = be32_to_cpu(*((__be32 *) (prop->value + 4))); if
> >> >> + (prop_value == 2)
> >> >> + priv->antenna_rx = ANTENNA_RX_2;
> >> >> + }
> >> >> + }
> >> >> +
> >> >> + priv->pwr_node = of_find_node_by_name(priv->dt_node,
> >> >> + "marvell,powertable");
> >> >> +#endif
> >> >> +}
> >> >
> >> > AFAICT, there's no documentation for these DT bindings. The mwlwifi
> >> > node and the marvell,powertable both need full documentation in
> >> > Documentation/devicetree/bindings/... .
> >> >
> >> > Frankly I have a feeling I'm going to need these DT nodes and I'd
> >> > like to not have to guess-and-check based on the code.
> >>
> >> Please use ieee80211-freq-limit:
> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commi
> >> t/?id=b3
> >> 30b25eaabda00d74e47566d9200907da381896
> >>
> >> Most likely with wiphy_read_of_freq_limits helper:
> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commi
> >> t/?id=e6
> >> 91ac2f75b69bee743f0370d79454ba4429b175
> >
> > I already replied meaning of these parameters:
> > is used to disable 2g band.
> > is used to disable 5g band.
> > is used to specify antenna number (if default number
> is suitable for you, there is no need to use this parameter).
> > should not be used for chip with device power table.
> In fact, this parameter should not be used any more.
> >
>
> David, I think you're not understanding the comment, or at least that's what
> it
> looks like to me. Yes, you did reply as to the meaning.
> And, your reply, while informative, didn't tell us you understood and were
> willing to fix the problem. I doubt you meant it this way, but it feels
> defensive
> and like a brush-off.
>
> First off, you will still have to document any DT bindings you're adding. Just
> because you answer the question in the review doesn't mean you're not
> responsible for doing so.
>
> And second off, I think that Rafal (and sorry about my spelling, looks like
> there's some sort of accent on the l that I don't know how to make my
> keyboard do) is saying: there's already some generic bindings that can be used
> to disable the 2g or 5g bands. Granted they're even newer than your patch,
> but I do think if said bindings exist and are appropriate, you should use
> them.
>
> - Steve
These parameters are marvell proprietary parameters, I don't think it should be
added to DT bindings.
BTW, and are only used for mwlwifi to report
supported bands, it is not related to limitation of frequency.
RE: [PATCH v9] Add new mac80211 driver mwlwifi.
> From: Rafał Miłecki [mailto:zaj...@gmail.com] worte: > Sent: Wednesday, February 08, 2017 3:24 PM > On 8 February 2017 at 07:30, David Lin wrote: > > steve.deros...@gmail.com [mailto:steve.deros...@gmail.com] wrote: > >> On Tue, Feb 7, 2017 at 10:15 PM, David Lin wrote: > >> >> Rafał Miłecki [mailto:zaj...@gmail.com] wrote: > >> >> Please use ieee80211-freq-limit: > >> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/co > >> >> mmi > >> >> t/?id=b3 > >> >> 30b25eaabda00d74e47566d9200907da381896 > >> >> > >> >> Most likely with wiphy_read_of_freq_limits helper: > >> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/co > >> >> mmi > >> >> t/?id=e6 > >> >> 91ac2f75b69bee743f0370d79454ba4429b175 > >> > > >> > I already replied meaning of these parameters: > >> > is used to disable 2g band. > >> > is used to disable 5g band. > >> > is used to specify antenna number (if default > >> > number > >> is suitable for you, there is no need to use this parameter). > >> > should not be used for chip with device power > table. > >> In fact, this parameter should not be used any more. > >> > > >> > >> David, I think you're not understanding the comment, or at least > >> that's what it looks like to me. Yes, you did reply as to the meaning. > >> And, your reply, while informative, didn't tell us you understood and > >> were willing to fix the problem. I doubt you meant it this way, but > >> it feels defensive and like a brush-off. > >> > >> First off, you will still have to document any DT bindings you're > >> adding. Just because you answer the question in the review doesn't > >> mean you're not responsible for doing so. > >> > >> And second off, I think that Rafal (and sorry about my spelling, > >> looks like there's some sort of accent on the l that I don't know how > >> to make my keyboard do) is saying: there's already some generic > >> bindings that can be used to disable the 2g or 5g bands. Granted > >> they're even newer than your patch, but I do think if said bindings exist > >> and > are appropriate, you should use them. > >> > >> - Steve > > > > These parameters are marvell proprietary parameters, I don't think it should > be added to DT bindings. > > Steve is correct. > > You have to document new properties, just because they are Marvell specific > doesn't change anything. You just document them in a proper place. > All right. I will do that. > > > BTW, and are only used for mwlwifi to report > supported bands, it is not related to limitation of frequency. > > How reporting a single band doesn't limit reported frequencies? You can > achieve exactly the same using generic property, so there is no place for > Marvell specific ones. > > In fact there were drivers of 3 vendors requiring band/freq-related > description > in DT: Broadcom, Marvell & Mediatek. This property was discussed & designed > to support all limitation cases we found possible to make it usable with > (hopefully) all drivers. > I only need simple way to disable 2g or 5g band. I will follow your suggestion to document these marvell proprietary parameters. Thanks, David > -- > Rafał
RE: [PATCH v9] Add new mac80211 driver mwlwifi.
> From: Rafał Miłecki [mailto:zaj...@gmail.com] worte: > On 8 February 2017 at 08:28, David Lin wrote: > >> From: Rafał Miłecki [mailto:zaj...@gmail.com] worte: > >> Sent: Wednesday, February 08, 2017 3:24 PM On 8 February 2017 at > >> 07:30, David Lin wrote: > >> > steve.deros...@gmail.com [mailto:steve.deros...@gmail.com] wrote: > >> >> On Tue, Feb 7, 2017 at 10:15 PM, David Lin wrote: > >> >> >> Rafał Miłecki [mailto:zaj...@gmail.com] wrote: > >> >> >> Please use ieee80211-freq-limit: > >> >> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git > >> >> >> /co > >> >> >> mmi > >> >> >> t/?id=b3 > >> >> >> 30b25eaabda00d74e47566d9200907da381896 > >> >> >> > >> >> >> Most likely with wiphy_read_of_freq_limits helper: > >> >> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git > >> >> >> /co > >> >> >> mmi > >> >> >> t/?id=e6 > >> >> >> 91ac2f75b69bee743f0370d79454ba4429b175 > >> >> > > >> >> > I already replied meaning of these parameters: > >> >> > is used to disable 2g band. > >> >> > is used to disable 5g band. > >> >> > is used to specify antenna number (if > >> >> > default number > >> >> is suitable for you, there is no need to use this parameter). > >> >> > should not be used for chip with device > >> >> > power > >> table. > >> >> In fact, this parameter should not be used any more. > >> >> > > >> >> > >> >> David, I think you're not understanding the comment, or at least > >> >> that's what it looks like to me. Yes, you did reply as to the meaning. > >> >> And, your reply, while informative, didn't tell us you understood > >> >> and were willing to fix the problem. I doubt you meant it this > >> >> way, but it feels defensive and like a brush-off. > >> >> > >> >> First off, you will still have to document any DT bindings you're > >> >> adding. Just because you answer the question in the review doesn't > >> >> mean you're not responsible for doing so. > >> >> > >> >> And second off, I think that Rafal (and sorry about my spelling, > >> >> looks like there's some sort of accent on the l that I don't know > >> >> how to make my keyboard do) is saying: there's already some > >> >> generic bindings that can be used to disable the 2g or 5g bands. > >> >> Granted they're even newer than your patch, but I do think if said > >> >> bindings exist and > >> are appropriate, you should use them. > >> >> > >> >> - Steve > >> > > >> > These parameters are marvell proprietary parameters, I don't think > >> > it should > >> be added to DT bindings. > >> > >> Steve is correct. > >> > >> You have to document new properties, just because they are Marvell > >> specific doesn't change anything. You just document them in a proper > place. > >> > > > > All right. I will do that. > > > >> > >> > BTW, and are only used for mwlwifi to > >> > report > >> supported bands, it is not related to limitation of frequency. > >> > >> How reporting a single band doesn't limit reported frequencies? You > >> can achieve exactly the same using generic property, so there is no > >> place for Marvell specific ones. > >> > >> In fact there were drivers of 3 vendors requiring band/freq-related > >> description in DT: Broadcom, Marvell & Mediatek. This property was > >> discussed & designed to support all limitation cases we found > >> possible to make it usable with > >> (hopefully) all drivers. > >> > > > > I only need simple way to disable 2g or 5g band. I will follow your > > suggestion > to document these marvell proprietary parameters. > > Seriously? Refusing to use generic binding because you think marvell,5ghz; is > simpler than ieee80211-freq-limit = <2402000 2482000>; (not to mention your > property seems reversed!)? > > I don't know how else to explain this to you. We don't want duplicated > properties where one can work. Just use existing one. Don't add new one even > if documented. > All right. I will check and let patch v10 to use it. For previous parameters, they will only be used by previous OpenWrt projects. Thanks, David > -- > Rafał
RE: [v8] Add new mac80211 driver mwlwifi.
Kalle Valo [mailto:kv...@codeaurora.org] wrote:. > > David Lin wrote: > > This patch provides the mwlwifi driver for Marvell 8863, 8864 and 8897 > > chipsets. > > This driver was developed as part of the openwrt.org project to > > support Linksys WRT1900AC and is maintained on > https://github.com/kaloz/mwlwifi. > > > > The mwlwifi driver differs from existing mwifiex driver: > > o mwlwifi is a "softmac driver" using the kernel mac802.11 subsystem > > to provide full AP/Wireless Bridge functionality (routers). > > o mwifiex is a "fullmac driver" which provides a comprehensive set of > > client functions (laptops/embedded devices) o only mwlwifi supports > > Marvell AP chip 886X series > > > > NOTE: Users with Marvell 88W8897 chipsets currently should enable > > (CONFIG=Y or M) either CONFIG_MWIFIEX or CONFIG_MWLWIFI, NOT > BOTH. > > > > mwlwifi driver leveraged code from existing MWL8K driver in the > > following areas: > > - 802.11n setting for mac80211 > > - Functions needed to hook up to mac80211 > > - Interactions with mac80211 to establish BA streams > > - Partial firmware APIs, some data fields > > - Method to pass Rx packets to mac80211 except 11ac rates > > > > In addition, mwlwifi driver supports: > > - future scalability and future development (refactored source code) > > - Marvell 802.11ac chipsets, including combo BT devices > > - 802.11ac related settings and functions > > - concurrent AP+STA functionalities with single firmware per chip > > - firmware APIs for the supported chipset > > - communicating new mac80211 settings to firmware > > - Different TX/RX datapath where applicable > > - A-MSDU and A-MPDU > > - Refined the code to establish BA streams > > > > Signed-off-by: David Lin > > I applied this to the pending branch for kbuild bot to run it's tests. > Thanks. > -- > Sent by pwcli > https://patchwork.kernel.org/patch/9201663/
[PATCH] mwlwifi: Initial submission of wireless driver to mainline
Hi all, Herewith, I am submitting the Linux driver for WRT1900AC. The work was initially developed as part of openwrt effort and maintained on https://github.com/kaloz/mwlwifi, thanks to the host where it is nurtured. Recently, I have also patched it to pass checkpatch.pl so that it can be qualified for mainline. This is still work in progress, with 8864 chipset more mature and tested, while 8897 for the similar use case is added recently. We will continue to maintain and enhance it. We hope the community will welcome this initial submission! Regards, David Lin (1): Add new mac80211 driver mwlwifi. drivers/net/wireless/Kconfig|1 + drivers/net/wireless/Makefile |2 + drivers/net/wireless/mwlwifi/Kconfig| 17 + drivers/net/wireless/mwlwifi/Makefile |9 + drivers/net/wireless/mwlwifi/mwl_debug.c| 207 ++ drivers/net/wireless/mwlwifi/mwl_debug.h| 118 + drivers/net/wireless/mwlwifi/mwl_dev.h | 465 drivers/net/wireless/mwlwifi/mwl_fwcmd.c| 3615 +++ drivers/net/wireless/mwlwifi/mwl_fwcmd.h| 184 ++ drivers/net/wireless/mwlwifi/mwl_fwdl.c | 218 ++ drivers/net/wireless/mwlwifi/mwl_fwdl.h | 29 + drivers/net/wireless/mwlwifi/mwl_mac80211.c | 877 +++ drivers/net/wireless/mwlwifi/mwl_mac80211.h | 32 + drivers/net/wireless/mwlwifi/mwl_main.c | 1095 drivers/net/wireless/mwlwifi/mwl_rx.c | 589 + drivers/net/wireless/mwlwifi/mwl_rx.h | 30 + drivers/net/wireless/mwlwifi/mwl_sysadpt.h | 64 + drivers/net/wireless/mwlwifi/mwl_tx.c | 892 +++ drivers/net/wireless/mwlwifi/mwl_tx.h | 34 + 19 files changed, 8478 insertions(+) create mode 100644 drivers/net/wireless/mwlwifi/Kconfig create mode 100644 drivers/net/wireless/mwlwifi/Makefile create mode 100644 drivers/net/wireless/mwlwifi/mwl_debug.c create mode 100644 drivers/net/wireless/mwlwifi/mwl_debug.h create mode 100644 drivers/net/wireless/mwlwifi/mwl_dev.h create mode 100644 drivers/net/wireless/mwlwifi/mwl_fwcmd.c create mode 100644 drivers/net/wireless/mwlwifi/mwl_fwcmd.h create mode 100644 drivers/net/wireless/mwlwifi/mwl_fwdl.c create mode 100644 drivers/net/wireless/mwlwifi/mwl_fwdl.h create mode 100644 drivers/net/wireless/mwlwifi/mwl_mac80211.c create mode 100644 drivers/net/wireless/mwlwifi/mwl_mac80211.h create mode 100644 drivers/net/wireless/mwlwifi/mwl_main.c create mode 100644 drivers/net/wireless/mwlwifi/mwl_rx.c create mode 100644 drivers/net/wireless/mwlwifi/mwl_rx.h create mode 100644 drivers/net/wireless/mwlwifi/mwl_sysadpt.h create mode 100644 drivers/net/wireless/mwlwifi/mwl_tx.c create mode 100644 drivers/net/wireless/mwlwifi/mwl_tx.h -- 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] mwlwifi: Initial submission of wireless driver to mainline
> On 06/03/2015 09:55 PM, David Lin wrote: > > Hi all, > > > > Herewith, I am submitting the Linux driver for WRT1900AC. The work > > was initially developed as part of openwrt effort and maintained on > > https://github.com/kaloz/mwlwifi, thanks to the host where it is > > nurtured. > > > > Recently, I have also patched it to pass checkpatch.pl so that it can > > be qualified for mainline. > > > > This is still work in progress, with 8864 chipset more mature and > > tested, while 8897 for the similar use case is added recently. We will > > continue to maintain and enhance it. > > > > We hope the community will welcome this initial submission! > > Do you have any notes on what features this driver/NIC provides? > > Does it require firmware, and if so, is source available? > Support both 11b/g/a/n/ac AP mode, and STA mode (though the primary use case is AP). Driver matches most features in mac80211 and hostapd/supplicant. All basic securities mode have been validated. Yes, it will require firmware microcode. Matching "binary" is reprovided as a separate .bin file(s) for each chip. Source codes of chip-dependent microcode will not be provided. Thanks, David -- 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] mwlwifi: Initial submission of wireless driver to mainline
> On 06/05/2015 12:29 PM, Ben Greear wrote: > On 06/04/2015 07:41 PM, David Lin wrote: > >> On 06/03/2015 09:55 PM, David Lin wrote: > >>> Hi all, > >>> > >>> Herewith, I am submitting the Linux driver for WRT1900AC. The > work > >>> was initially developed as part of openwrt effort and maintained on > >>> https://github.com/kaloz/mwlwifi, thanks to the host where it is > >>> nurtured. > >>> > >>> Recently, I have also patched it to pass checkpatch.pl so that it > >>> can be qualified for mainline. > >>> > >>> This is still work in progress, with 8864 chipset more mature and > >>> tested, while 8897 for the similar use case is added recently. We > >>> will continue to maintain and enhance it. > >>> > >>> We hope the community will welcome this initial submission! > >> > >> Do you have any notes on what features this driver/NIC provides? > >> > >> Does it require firmware, and if so, is source available? > >> > > Support both 11b/g/a/n/ac AP mode, and STA mode (though the primary use > > case is AP). Driver matches most features in mac80211 and > > hostapd/supplicant. All basic securities mode have been validated. > > > > Yes, it will require firmware microcode. Matching "binary" is reprovided as > > a > > separate .bin file(s) for each chip. Source codes of chip-dependent > > microcode > > will not be provided. > > Can it do multiple virtual stations (and other virtual device combinations)? > > With ath10k, for instance, there is a complex firmware between the host and > the NIC hardware, so any bugs and limitations in the firmware make it almost > impossible to add interesting features or fix bugs w/out being able to modify > the firmware source. > > Any comments on how 'thick' the marvell firmware is? > Yes, the partitioning technically allows multiple logical interfaces. However, this open source driver is not yet developed for many combinations, yet. It is hard to comment about "thickness" of the firmware :) All (most?) 802.11 mgmt packets are handled in host driver. Think of microcode as part of hardware or controlling/abstracting the hardware. Thanks, David > > -- > Ben Greear > Candela Technologies Inc http://www.candelatech.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
RE: [PATCH] mwlwifi: Initial submission of wireless driver to mainline
> On 06/06/2015 6:01 AM, Alexis Green wrote:
> Are there plans to add support for 802.11s mesh point mode?
>
Yes, it will be worked on in the future.
Thanks,
David
>
> On Fri, Jun 5, 2015 at 8:09 AM, David Lin wrote:
> >> On 06/05/2015 12:29 PM, Ben Greear wrote:
> >> On 06/04/2015 07:41 PM, David Lin wrote:
> >> >> On 06/03/2015 09:55 PM, David Lin wrote:
> >> >>> Hi all,
> >> >>>
> >> >>> Herewith, I am submitting the Linux driver for WRT1900AC. The
> >> work
> >> >>> was initially developed as part of openwrt effort and maintained
> >> >>> on https://github.com/kaloz/mwlwifi, thanks to the host where it
> >> >>> is nurtured.
> >> >>>
> >> >>> Recently, I have also patched it to pass checkpatch.pl so that
> >> >>> it can be qualified for mainline.
> >> >>>
> >> >>> This is still work in progress, with 8864 chipset more mature
> >> >>> and tested, while 8897 for the similar use case is added
> >> >>> recently. We will continue to maintain and enhance it.
> >> >>>
> >> >>> We hope the community will welcome this initial submission!
> >> >>
> >> >> Do you have any notes on what features this driver/NIC provides?
> >> >>
> >> >> Does it require firmware, and if so, is source available?
> >> >>
> >> > Support both 11b/g/a/n/ac AP mode, and STA mode (though the primary
> >> > use case is AP). Driver matches most features in mac80211 and
> >> > hostapd/supplicant. All basic securities mode have been validated.
> >> >
> >> > Yes, it will require firmware microcode. Matching "binary" is
> >> > reprovided as a separate .bin file(s) for each chip. Source codes
> >> > of chip-dependent microcode will not be provided.
> >>
> >> Can it do multiple virtual stations (and other virtual device
> >> combinations)?
> >>
> >> With ath10k, for instance, there is a complex firmware between the
> >> host and the NIC hardware, so any bugs and limitations in the
> >> firmware make it almost impossible to add interesting features or fix
> >> bugs w/out being able to modify the firmware source.
> >>
> >> Any comments on how 'thick' the marvell firmware is?
> >>
> > Yes, the partitioning technically allows multiple logical interfaces.
> > However,
> this open source driver is not yet developed for many combinations, yet.
> >
> > It is hard to comment about "thickness" of the firmware :) All (most?)
> > 802.11
> mgmt packets are handled in host driver. Think of microcode as part of
> hardware or controlling/abstracting the hardware.
> >
> > Thanks,
> > David
> >>
> >> --
> >> Ben Greear
> >> Candela Technologies Inc http://www.candelatech.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
N�r��yb�X��ǧv�^�){.n�+{��*ޕ�,�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj"��!�i
RE: [PATCH] Add new mac80211 driver mwlwifi.
> On Saturday, June 06, 2015 9:43 PM, Johannes Berg wrote:
>
> Hi David, all,
>
> I'm not really in the chain here, but I figured I'd review your driver
> nonetheless. I'll want to take a closer look at some things, but for now
> here's a
> bit of a review just of the code.
>
> Can you perhaps explain how the STA/AP firmware separation works? I've not
> really managed to figure that out - admittedly with not all that much effort
> though.
>
Hi Johannes,
Questions:
1. That's interesting, why does a PCI(e) driver need OF?
This driver will accept parameters in DTS file for band control, antenna
setting and power table, to cater for different boards combination inside a
system.
2. Can you perhaps explain how the STA/AP firmware separation works?
For this driver, the AP/STA mode will use the same single firmware binary, so
there is no “separation” per se. The firmware will support AP/STA mode.
3. Does this driver has any relation to mwifiex?
Mwifiex is driver for Firmware-based MLME. It interfaces with firmware with
802.3 packets. Mwlwifi is the Host Soft AP/STA driver that works with mac80211.
I will be working on the following changes as you suggested:
1. Remove mwl from filename (mwl_xxx.x -> xxx.x).
2. Remove comment for the purpose of the code block and take off static
function pre-declaration.
3. Remove file debug.c and debug.h.
4. Typo error, unnecessary comment, prefer comment style and use BIT MACRO
instead of constant.
5. Move isr related functions to isr.c and isr.h.
6. Remove MACRO for SPIN LOCK and let spin lock be attached to related data
structure.
7. For firmware related structure, use __le16 for u16 and __le32 for u32. Add
compiler control “-D__CHECK_ENDIAN__” to help to check endian problem.
8. For firmware related structure, take off bit field.
We recommend that the followings should not be changed at current stage:
1. Directory name “mwlwifi”.
This is to be consistent with some predecessor. For example, Intel uses
iwlwifi, realtek uses rtlwifi. We do not see a need to change it. This will
make sure we keep the original project mwlwifi on openwrt folder remain intact,
while we continue to maintain them the same way/pace.
2. Interface with F/W.
F/W used by this driver is also used by other marvell’s drivers.
3. AMPDU related code.
It has been well tested and leveraged from mwl8k. We may enhance it in future,
but please accept the current code status for now.
After modification is done, I will come out [PATCH v2] for you to review again.
Thanks,
David
>
> On Thu, 2015-06-04 at 04:57 +, David Lin wrote:
>
> > drivers/net/wireless/mwlwifi/Kconfig| 17 +
>
> Does this driver has any relation to mwifiex? Perhaps the same maintainer
> team, etc.? Could consider moving all the Marvell drivers into one directory,
> but not really necessary I guess.
>
> Out of curiosity - why "mwlwifi" and not just "mwl" or similar? :)
>
> Also - consider adding a MAINTAINERS entry for this driver.
>
> > drivers/net/wireless/mwlwifi/mwl_debug.c| 207 ++
>
> The mwl_ prefix doesn't really do much for this driver (especially since it's
> used for all files) -- I'd consider removing it.
>
> > @@ -284,5 +284,6 @@ source "drivers/net/wireless/zd1211rw/Kconfig"
> > source "drivers/net/wireless/mwifiex/Kconfig"
> > source "drivers/net/wireless/cw1200/Kconfig"
> > source "drivers/net/wireless/rsi/Kconfig"
> > +source "drivers/net/wireless/mwlwifi/Kconfig"
>
> Perhaps, just like with the directory structure, we should also consider
> having
> per-vendor Kconfig structure, like Ethernet drivers have now.
>
> Then again, unless we decide we want to do this for all drivers and all
> devices
> it doesn't really do much.
>
> > + depends on PCI && MAC80211
> > + select FW_LOADER
> > + select OF
>
> That's interesting, why does a PCI(e) driver need OF?
>
> > +/* CONSTANTS AND MACROS
> > +*/
>
> that one-line comment style (also in other places below, and perhaps other
> files) is a but strange
>
> > +#define PRT_8BYTES "0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x
> 0x%02x 0x%02x\n"
>
> Do you really need the "0x" in here? Otherwise it'd be much nicer to use %ph
> instead of macros.
>
> > +void mwl_debug_prt(u32 classlevel, const char *func, const char
> > +*format, ...) {
>
> > + debug_string = kmalloc(1024, GFP_ATOMIC);
> > +
> > + if (!debug_string)
> > + return;
>
> That seems a bit questionable - when memory allocations start failing is one
> of
> those places where you really want debug output ...
>
>
RE: [PATCH v2] Add new mac80211 driver mwlwifi.
> On Wed, 2015-06-17 at 3:28 PM Johannes Berg wrote:
>
> On Wed, 2015-06-17 at 06:06 +0000, David Lin wrote:
>
> (not really reading all of this right now, just two small comments)
>
> > +struct mwl_tx_desc {
> > + u8 data_rate;
> > + u8 tx_priority;
> > + __le16 qos_ctrl;
> > + __le32 pkt_ptr;
> > + __le16 pkt_len;
> > + u8 dest_addr[ETH_ALEN];
> > + __le32 pphys_next;
> > + __le32 sap_pkt_info;
> > + __le32 rate_info;
> > + u8 type;
> > + u8 xmit_control; /* bit 0: use rateinfo, bit 1: disable ampdu */
> > + __le16 reserved;
> > + __le32 tcpack_sn;
> > + __le32 tcpack_src_dst;
> > + struct sk_buff *psk_buff;
> > + struct mwl_tx_desc *pnext;
> > + u8 reserved1[2];
> > + u8 packet_info;
> > + u8 packet_id;
> > + __le16 packet_len_and_retry;
> > + __le16 packet_rate_info;
> > + u8 *sta_info;
> > + __le32 status;
> > +} __packed;
>
> This ... cannot be right. You can't have a pointer in a structure that's used
> by
> firmware, and the __leXX and __packed indicates that it's used by firmware.
> But pointer size is variable, so *clearly* this cannot be correct.
>>
> > +struct mwl_rx_desc {
> > + __le16 pkt_len; /* total length of received data */
> > + __le16 rate; /* receive rate information
> */
> > + __le32 pphys_buff_data; /* physical address of payload data */
> > + __le32 pphys_next; /* physical address of next RX desc */
> > + __le16 qos_ctrl; /* received QosCtrl field variable*/
> > + __le16 ht_sig2; /* like name states
> */
> > + __le32 hw_rssi_info;
> > + __le32 hw_noise_floor_info;
> > + u8 noise_floor;
> > + u8 reserved[3];
> > + u8 rssi; /* received signal strengt indication */
> > + u8 status; /* status field containing USED bit */
> > + u8 channel; /* channel this pkt was received on
> */
> > + u8 rx_control; /* the control element of the desc*/
> > + /* above are 32bits aligned and is same as FW, RxControl put at end
> > +* for sync
> > +*/
> > + struct sk_buff *psk_buff;/* associated sk_buff for Linux */
> > + void *pbuff_data;/* virtual address of payload data*/
> > + struct mwl_rx_desc *pnext; /* virtual address of next RX desc*/
> > +} __packed;
>
> Same here. If you really need to have a firmware and driver struct as
> indicated by the comment, you should probably have
>
>
> struct mwl_rx_info {
> /* firmware info */
> } __packed;
>
> struct mwl_rx_desc {
> struct mwl_rx_info info;
> /* driver info */
> };
>
> (note the lack of __packed also, __packed often makes accesses far less
> efficient)
>
> > +struct mwl_desc_data {
> > + dma_addr_t pphys_tx_ring; /* ptr to first TX desc (phys.)
> */
> > + struct mwl_tx_desc *ptx_ring; /* ptr to first TX desc (virt.)*/
> > + struct mwl_tx_desc *pnext_tx_desc; /* next TX desc that can be used
> */
> > + struct mwl_tx_desc *pstale_tx_desc;/* the staled TX descriptor
> */
> > + dma_addr_t pphys_rx_ring; /* ptr to first RX desc (phys.)
> */
> > + struct mwl_rx_desc *prx_ring; /* ptr to first RX desc (virt.)*/
> > + struct mwl_rx_desc *pnext_rx_desc; /* next RX desc that can be used
> */
> > + unsigned int wcb_base; /* FW base offset for registers
> */
> > + unsigned int rx_desc_write;/* FW descriptor write position
> */
> > + unsigned int rx_desc_read; /* FW descriptor read position
> */
> > + unsigned int rx_buf_size; /* length of the RX buffers
> */
> > +} __packed;
>
> ditto - don't pack driver structs
>
> Also, you probably really should have two header files - one for the firmware
> structs and one for the driver structs - especially since you seem to be
> confusing the two!
>
As mentioned before, this is current interface with F/W, and this F/W is used
by other marvell's drivers, I can't change it.
>
> > +++ b/drivers/net/wireless/mwlwifi/fwcmd.c
>
> There are a lot of struct definitions here that you could move to the same
> header file.
>
All structures defined in this file are related to F/W commands, it should be
better let them be defined in this file.
>
> > +#if SYSADPT_NUM_OF_DESC_DATA > 3
>
> how does that get set?
>
SYSADPT_NUM_OF_DESC_DATA is define in sysadpt
RE: [PATCH v2] Add new mac80211 driver mwlwifi.
> Johannes Berg wrote: > > On Wed, 2015-06-17 at 08:07 +0000, David Lin wrote: > > > > Also, you probably really should have two header files - one for the > > > firmware structs and one for the driver structs - especially since > > > you seem to be confusing the two! > > > > > As mentioned before, this is current interface with F/W, and this F/W > > is used by other marvell's drivers, I can't change it. > > You're misunderstanding. I'm not asking you to change the interface. I'm just > asking you to make sure you know which part is firmware interface and which > isn't. Clearly, pointers *cannot* be firmware interface - if there's something > "cookie-like" that you use as pointers you at least need to make sure you know > how long this field is (32 or 64 bits)... > Yes, we know this kind of issue. We will enhance it in the future. > > > All structures defined in this file are related to F/W commands, it > > should be better let them be defined in this file. > > Given your own confusion on what's firmware API and what isn't, I'm not so > sure about that ... > I am sure all structures defined in this file is related to firmware command. > > johannes
RE: [PATCH v2] Add new mac80211 driver mwlwifi.
> Dan Williams wrote: > > On Wed, 2015-06-17 at 08:34 +0000, David Lin wrote: > > > Johannes Berg wrote: > > > > > > On Wed, 2015-06-17 at 08:07 +, David Lin wrote: > > > > > > > > Also, you probably really should have two header files - one for > > > > > the firmware structs and one for the driver structs - especially > > > > > since you seem to be confusing the two! > > > > > > > > > As mentioned before, this is current interface with F/W, and this > > > > F/W is used by other marvell's drivers, I can't change it. > > > > > > You're misunderstanding. I'm not asking you to change the interface. > > > I'm just asking you to make sure you know which part is firmware > > > interface and which isn't. Clearly, pointers *cannot* be firmware > > > interface - if there's something "cookie-like" that you use as > > > pointers you at least need to make sure you know how long this field is > > > (32 > or 64 bits)... > > > > > Yes, we know this kind of issue. We will enhance it in the future. > > > > > > > All structures defined in this file are related to F/W commands, > > > > it should be better let them be defined in this file. > > > > > > Given your own confusion on what's firmware API and what isn't, I'm > > > not so sure about that ... > > > > > I am sure all structures defined in this file is related to firmware > > command. > > They may well be related, but that doesn't necessarily mean that the driver > side cannot be clearer about what fields of a struct are actually read by the > firmware, and what fields are private to the host implementation as a data > stash (eg, 'priv'-type stuff). Redoing the structs like Johannes suggested > originally would make that crystal-clear... > > Dan Yes, we plan to modify the code as Johannes suggested and come out PATCH v3 for review. Thanks. David
RE: [PATCH v3] Add new mac80211 driver mwlwifi.
> Johannes Berg wrote: > On Thu, 2015-06-25 at 03:48 +, David Lin wrote: > > Signed-off-by: David Lin > > This really needs a commit message, like saying what chips it supports, > perhaps where to find them, and similar. > O.K. We will add it. > > > drivers/net/wireless/Kconfig|1 + > > drivers/net/wireless/Makefile |2 + > > drivers/net/wireless/mwlwifi/Kconfig| 17 + > > drivers/net/wireless/mwlwifi/Makefile |9 + > > drivers/net/wireless/mwlwifi/dev.h | 435 ++ > > drivers/net/wireless/mwlwifi/fwcmd.c| 2328 > +++ > > drivers/net/wireless/mwlwifi/fwcmd.h| 175 +++ > > drivers/net/wireless/mwlwifi/fwdl.c | 183 +++ > > drivers/net/wireless/mwlwifi/fwdl.h | 27 + > > drivers/net/wireless/mwlwifi/hostcmd.h | 753 ++ > > drivers/net/wireless/mwlwifi/isr.c | 148 ++ > > drivers/net/wireless/mwlwifi/isr.h | 26 + > > drivers/net/wireless/mwlwifi/mac80211.c | 743 ++ > > drivers/net/wireless/mwlwifi/mac80211.h | 25 + > > drivers/net/wireless/mwlwifi/main.c | 858 > > drivers/net/wireless/mwlwifi/rx.c | 523 +++ > > drivers/net/wireless/mwlwifi/rx.h | 25 + > > drivers/net/wireless/mwlwifi/sysadpt.h | 67 + > > drivers/net/wireless/mwlwifi/tx.c | 836 +++ > > drivers/net/wireless/mwlwifi/tx.h | 28 + > > 20 files changed, 7209 insertions(+) > > You're missing a MAINTAINERS entry. > We will add it. > > > +++ b/drivers/net/wireless/mwlwifi/Kconfig > > @@ -0,0 +1,17 @@ > > +config MWLWIFI > > + tristate "Marvell Wireless WiFi driver (mwlwifi)" > > + depends on PCI && MAC80211 && MWIFIEX_PCIE=n > > Uh, what's with the exclusion of MWIFIEX_PCIE? Couldn't use a different PCI > ID? > I really think you need to get rid of this, otherwise people can't even > *build-test* their wifi changes fully. > Both the drivers are operable for the same chip part number 8897 and its modules (boards) manufactured out there. The PCI Device ID is a fixed value (by chip/manufacturer) on those modules. As said earlier, the two drivers are for mac80211 host mac, and firmware mac respectively. They serve different purposes. Users need to choose which driver they want to use for the same wifi hardware, and they cannot use both of them at the same time. The mwifiex is more widely used now and sta centric. We do not want to break it. I am not sure but I believe they are also multiple flavors of driver for some other chips. Any advice how they are handled, considering that changing the hardware/board is not an option? I guess we want the community to be better served using the same hardware. Opinion from others are welcomed. > > > +++ b/drivers/net/wireless/mwlwifi/Makefile > > @@ -0,0 +1,9 @@ > > +obj-$(CONFIG_MWLWIFI) += mwlwifi.o > > + > > +mwlwifi-objs += main.o > > +mwlwifi-objs += mac80211.o > > +mwlwifi-objs += fwdl.o > > +mwlwifi-objs += fwcmd.o > > +mwlwifi-objs += tx.o > > +mwlwifi-objs += rx.o > > +mwlwifi-objs += isr.o > > Please add > > ccflags-y += -D__CHECK_ENDIAN__ > > to this file to always flag sparse errors. You have checked with sparse, > right? > > Ok ... you clearly haven't. Please add the line above, and fix up the results. > Also, you really need to try building this driver on 64-bit, it reports a good > number of warnings. > > If you're up to it, also try running smatch on it, it reports a number of more > warnings that sparse misses, like this one: > > > mwl_rx_ring_init() warn: variable dereferenced before check > 'rx_hndl->psk_buff' (see line 108) > Although "-D__CHECK_ENDIAN__" does not put into Makefile, I had used "C=2 CF="-D__CHECK_ENDIAN__" to check warnings for endian and remove these warnings. If you think we need to add this to Makefile and make sure zero warning messages, we will do that (only for sparser). BTW, is it possible for you to give us all your comments based on this patch? So we can include these modifications in PATCH v4. Thanks. > > This is going to be only a very superficial review until the driver builds > cleanly. > I do think the firmware API stuff I was concerned about earlier looks better > now though > > I think you went a bit overbaord with wiphy_info(), a number of those (like > the > beacon info one) surely are more debug messages than operational messages > that should be sho
RE: [PATCH v3] Add new mac80211 driver mwlwifi.
> Johannes Berg wrote: > > On Fri, 2015-06-26 at 01:25 +0000, David Lin wrote: > > > Both the drivers are operable for the same chip part number 8897 and > > its modules (boards) manufactured out there. The PCI Device ID is a > > fixed value (by chip/manufacturer) on those modules. As said earlier, > > the two drivers are for mac80211 host mac, and firmware mac > > respectively. They serve different purposes. Users need to choose > > which driver they want to use for the same wifi hardware, and they > > cannot use both of them at the same time. The mwifiex is more widely > > used now and sta centric. We do not want to break it. > > > > I am not sure but I believe they are also multiple flavors of driver > > for some other chips. Any advice how they are handled, considering > > that changing the hardware/board is not an option? I guess we want the > > community to be better served using the same hardware. Opinion from > > others are welcomed. > > Hmm, interesting, I didn't realize it was the same device. > > Perhaps it simply shouldn't conflict at build time, and you can recommend to > blacklist one or the other? Or, in fact, return an error from probe if you > can't > find the correct firmware file, so the other driver would be attempted? > Yes, current setting will only allow user to choose one of these two drivers at build time. > > > Although "-D__CHECK_ENDIAN__" does not put into Makefile, I had used > > "C=2 CF="-D__CHECK_ENDIAN__" to check warnings for endian and remove > > these warnings. > > Ok. Then I guess my issues were mostly due to 64-bit system. > I will try to build the driver under 64-bit system. > > > If you think we need to add this to Makefile and make sure zero > > warning messages, we will do that (only for sparser). > > Well it just helps not forget adding CF=-D... to the command line, it's > equivalent. I just prefer it in a sense since then if people run with > C=1 or C=2 they get that part for free, and since the driver is clean to start > with then that's a good thing to do I think. > Got it. > > I'll try to take a closer look later. > > Johannes > David, Thanks
RE: [PATCH v4] Add new mac80211 driver mwlwifi.
> Joe Perches wrote:
>
> On Tue, 2015-06-30 at 01:49 +0000, David Lin wrote:
> > The Linux driver for WRT1900AC. The work was initially developed as
> > part of openwrt effort and maintained on https://github.com/kaloz/mwlwifi.
>
> trivia:
>
> Please add terminating newlines to logging messages.
> This prevents interleaving output from multiple threads.
>
> > diff --git a/drivers/net/wireless/mwlwifi/fwcmd.c
> > b/drivers/net/wireless/mwlwifi/fwcmd.c
> []
> > +static bool mwl_fwcmd_chk_adapter(struct mwl_priv *priv) {
> []
> > + if (regval == 0x) {
> > + wiphy_err(priv->hw->wiphy, "adapter is not existed");
>
> wiphy_err(priv->hw->wiphy, "adapter is not existed\n");
>
> > +static int mwl_fwcmd_wait_complete(struct mwl_priv *priv, unsigned
> > +short cmd) {
> []
> > + if (curr_iteration == 0) {
> > + wiphy_err(priv->hw->wiphy, "cmd 0x%04x=%s timed out",
>
> wiphy_err(priv->hw->wiphy, "cmd 0x%04x=%s timed out\n",
>
> etc...
>
Should be modified in next patch.
--
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 v4] Add new mac80211 driver mwlwifi.
> James Cameron wrote:
> On Tue, Jun 30, 2015 at 09:18:44AM -0500, Dan Williams wrote:
> > On Tue, 2015-06-30 at 10:21 +0200, Johannes Berg wrote:
> > > On Tue, 2015-06-30 at 01:49 +, David Lin wrote:
> > > > +++ b/drivers/net/wireless/mwlwifi/Kconfig
> > > > @@ -0,0 +1,17 @@
> > > > +config MWLWIFI
> > > > + tristate "Marvell Wireless WiFi driver (mwlwifi)"
> > > > + depends on PCI && MAC80211 && MWIFIEX_PCIE=n
> > >
> > > I still think you need to get rid of this so we can build-test this
> > > driver properly.
> >
> > The OLPC 8388 is another device that has two drivers, libertas and
> > libertas_tf.
>
> Also 8686.
>
> > I don't think there's any protection between then, you get whatever
> > gets loaded first by the kernel. In that case, I think the answer was
> > either (a) only put the driver you want onto the system, or
>
> Yes, for end-user.
>
> > (b) manually manage from userspace.
>
> Yes, for developer testing.
>
> > Given that this Marvell hardware is likely intended for more
> > customized use-cases (AP, embedded, etc?) perhaps this would be an
> > acceptable option for now...
> >
Yes, you are right. The two drivers (MWLWIFI and MWIFIEX) serve different
customized use cases:
- MWLWIFI is mac80211-based SoftMAC with the primary use case of AP/Wireless
Bridge
- MWIFIEX is fullmac firmware for embedded clients They can operate on same
chip part number, for example 8897 in this case, and that could cause
confusion. MWIFIEX has been in full production with some clients.
> > I tend to agree with Johannes here; the builder of the kernel can
> > certainly adjust CONFIG_MWLWIFI and CONFIG_MWIFIEX to fit their
> > scenario, including leaving both enabled.
> >
> > Dan
We can leave both selectable by developer testing as per your suggestion, and
assume users/integrators will know how to put the driver they want in their
system. We were warned about causing confusion hence the conditioning in
CONFIG. Do you feel there's no concern leaving both driver in, not checking
each other's presence? We can comply either way.
> >
> > > > + select FW_LOADER
> > > > + select OF
> > >
> > > This looks OK, though I get a very strange dependency loop warning
> > > from Kconfig here.
For the next patch, we will modify the code to still work even though the
target does not support DTS. So we can remove "select OF" from Kconfig file.
> > >
> > > Looks like the driver now builds almost cleanly with sparse/smatch
> > > on 64-bit.
> > >
> > > Two warnings remain, both are bugs:
> > >
> > > > writew(0x00, (void __iomem *)&priv->pcmd_buf[1]);
> > >
> > > cannot be right. This memory isn't __iomem, it's dma_alloc_coherent,
> > > so a simple write should be done.
> > >
Without this casting, C=2 will cause a warning message like this: "Warning:
incorrect type in argument 2 (different address spaces)"
> > > in mwl_rx_ring_init:
> > >
> > > > rx_hndl->psk_buff =
> > > >
> dev_alloc_skb(desc->rx_buf_size);
> > > >
> > > > if (skb_linearize(rx_hndl->psk_buff)) {
> > >
> > > *crash*. You also later check rx_hndl->psk_buff, but well after it
> > > already crashed.
> > >
> > > Also, this code sequence is utterly bogus. Please try to understand
> > > why and then remove it.
> > >
> > > You should also use paged RX since you're allocating *very large*
> > > buffe rs. We found that even alloc_pages(1) will fail eventually,
> > > you're doing an order-2 allocation here for every RX skb. At least
> > > used paged RX to get it down to order-1.
The code will be corrected to take care of error exception.
> > >
> > > 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
> >
> >
> > --
> > 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
>
> --
> James Cameron
> http://quozl.linux.org.au/
Thanks,
David
--
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 v4] Add new mac80211 driver mwlwifi.
> Johannes Berg wrote: > On Wed, 2015-07-01 at 14:42 +, David Lin wrote: > > > We can leave both selectable by developer testing as per your > > suggestion, and assume users/integrators will know how to put the > > driver they want in their system. We were warned about causing > > confusion hence the conditioning in CONFIG. Do you feel there's no > > concern leaving both driver in, not checking each other's presence? > > We can comply either way. > > I think you should just leave both selectable. > OK, we will leave both of them selectable, and provide additional “NOTE” (warning) in the Kconfig HELP message. > > > > > > This looks OK, though I get a very strange dependency loop > > > > > warning from Kconfig here. > > > > For the next patch, we will modify the code to still work even though > > the target does not support DTS. So we can remove "select OF" from > > Kconfig file. > > If the driver needs OF don't bother - the dependency loop is very long and the > warning is rather strange. > > > > > > > writew(0x00, (void __iomem *)&priv->pcmd_buf[1]); > > > > > > > > > > cannot be right. This memory isn't __iomem, it's > > > > > dma_alloc_coherent, so a simple write should be done. > > > > > > > > > Without this casting, C=2 will cause a warning message like this: > > "Warning: incorrect type in argument 2 (different address spaces)" > > Yes, but the warning is correct and the cast is wrong. This isn't __iomem, > it's > simply mapped, so you should just do a > (u32 *)pcmd_buf[1] = 0; > > or something like that. The writev() to such a pointer cannot be right. > Understood. Thanks. Will make changes accordingly. > > > Johannes Thanks, David
[PATCH v8] Add new mac80211 driver mwlwifi.
PATCH v8 changes since PATCH v7: - Used scnprintf() to replace sprintf() for debugfs output messages to avoid overwriting buffer boundary. - Used mutex to replace spinlock for the protection of firmware command. - Used NL80211_BAND_ instead of IEEE80211_BAND_ (in order to work with updated mac80211). - Used usleep_range() instead of mdelay(). - Modified the code to work with new mac80211 API ampdu_action() and get peer AMSDU information from parameters of this function instead of peeking ADDBA related packets. - Removed BA stream if traffic is not heavy. - Removed version information. - Added DFS, WPS, WDS and thermal function. - Changed length of mac vht_mpdu from 7991 to 3895. David Lin (1): Add new mac80211 driver mwlwifi. MAINTAINERS |6 + drivers/net/wireless/marvell/Kconfig|1 + drivers/net/wireless/marvell/Makefile |1 + drivers/net/wireless/marvell/mwlwifi/Kconfig| 23 + drivers/net/wireless/marvell/mwlwifi/Makefile | 13 + drivers/net/wireless/marvell/mwlwifi/debugfs.c | 780 +++ drivers/net/wireless/marvell/mwlwifi/debugfs.h | 24 + drivers/net/wireless/marvell/mwlwifi/dev.h | 502 + drivers/net/wireless/marvell/mwlwifi/fwcmd.c| 2747 +++ drivers/net/wireless/marvell/mwlwifi/fwcmd.h| 214 ++ drivers/net/wireless/marvell/mwlwifi/fwdl.c | 186 ++ drivers/net/wireless/marvell/mwlwifi/fwdl.h | 25 + drivers/net/wireless/marvell/mwlwifi/hostcmd.h | 883 drivers/net/wireless/marvell/mwlwifi/isr.c | 172 ++ drivers/net/wireless/marvell/mwlwifi/isr.h | 27 + drivers/net/wireless/marvell/mwlwifi/mac80211.c | 719 ++ drivers/net/wireless/marvell/mwlwifi/main.c | 759 +++ drivers/net/wireless/marvell/mwlwifi/rx.c | 513 + drivers/net/wireless/marvell/mwlwifi/rx.h | 25 + drivers/net/wireless/marvell/mwlwifi/sysadpt.h | 83 + drivers/net/wireless/marvell/mwlwifi/thermal.c | 182 ++ drivers/net/wireless/marvell/mwlwifi/thermal.h | 40 + drivers/net/wireless/marvell/mwlwifi/tx.c | 1250 +++ drivers/net/wireless/marvell/mwlwifi/tx.h | 37 + 24 files changed, 9212 insertions(+) create mode 100644 drivers/net/wireless/marvell/mwlwifi/Kconfig create mode 100644 drivers/net/wireless/marvell/mwlwifi/Makefile create mode 100644 drivers/net/wireless/marvell/mwlwifi/debugfs.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/debugfs.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/dev.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/fwcmd.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/fwcmd.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/fwdl.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/fwdl.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/hostcmd.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/isr.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/isr.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/mac80211.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/main.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/rx.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/rx.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/sysadpt.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/thermal.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/thermal.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/tx.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/tx.h -- 1.9.3 -- 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 v8] Add new mac80211 driver mwlwifi.
> From: linux-wireless-ow...@vger.kernel.org > [mailto:linux-wireless-ow...@vger.kernel.org] On Behalf Of Baumann, > Christoph (C.) > Sent: Wednesday, June 29, 2016 9:45 PM > To: linux-wireless@vger.kernel.org > Subject: Re: [PATCH v8] Add new mac80211 driver mwlwifi. > > Hello David, > > I tried this new driver and it worked for me (board based on Renesas R-Car > H3). > But Inoticed that the data rates are asymmetrical (seen from the device): > * iperf -c (mainly TX): 598 Kbits/sec to 2.67 Mbits/sec > * iperf -s (mainly RX): 64.4 Mbits/sec to 69.2 Mbits/sec > Please check https://github.com/kaloz/mwlwifi to get latest firmware and configuration sample. If you still get problem, please open an issue on https://github.com/kaloz/mwlwifi. You can get HW, driver and firmware information via "cat /sys/kernel/debug/ieee80211/phy0/mwlwifi/info". Please specify your configuration and test environment. Thanks, David > > > Regards, > Christoph > -- > 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 -- 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 v5] Add new mac80211 driver mwlwifi.
>Jonas Gorski wrote: > > Hi, > > On Fri, Jul 3, 2015 at 8:10 AM, David Lin wrote: > > The Linux driver for WRT1900AC. The work was initially developed as > > part of openwrt effort and maintained on https://github.com/kaloz/mwlwifi. > > > > This is still work in progress, with 8864 chipset more mature and > > tested, while 8897 for the similar use case is added recently. > > > > Signed-off-by: David Lin > > --- > > drivers/net/wireless/Kconfig |1 + > > drivers/net/wireless/Makefile|2 + > > drivers/net/wireless/mwlwifi/Kconfig | 24 + > > drivers/net/wireless/mwlwifi/MAINTAINERS |5 + > > drivers/net/wireless/mwlwifi/Makefile| 11 + > > drivers/net/wireless/mwlwifi/dev.h | 435 ++ > > drivers/net/wireless/mwlwifi/fwcmd.c | 2278 > ++ > > drivers/net/wireless/mwlwifi/fwcmd.h | 175 +++ > > drivers/net/wireless/mwlwifi/fwdl.c | 183 +++ > > drivers/net/wireless/mwlwifi/fwdl.h | 27 + > > drivers/net/wireless/mwlwifi/hostcmd.h | 753 ++ > > drivers/net/wireless/mwlwifi/isr.c | 148 ++ > > drivers/net/wireless/mwlwifi/isr.h | 26 + > > drivers/net/wireless/mwlwifi/mac80211.c | 739 ++ > > drivers/net/wireless/mwlwifi/mac80211.h | 25 + > > drivers/net/wireless/mwlwifi/main.c | 856 +++ > > drivers/net/wireless/mwlwifi/rx.c| 519 +++ > > drivers/net/wireless/mwlwifi/rx.h| 25 + > > drivers/net/wireless/mwlwifi/sysadpt.h | 67 + > > drivers/net/wireless/mwlwifi/tx.c| 834 +++ > > drivers/net/wireless/mwlwifi/tx.h| 28 + > > 21 files changed, 7161 insertions(+) > > create mode 100644 drivers/net/wireless/mwlwifi/Kconfig > > create mode 100644 drivers/net/wireless/mwlwifi/MAINTAINERS > > create mode 100644 drivers/net/wireless/mwlwifi/Makefile > > create mode 100644 drivers/net/wireless/mwlwifi/dev.h > > create mode 100644 drivers/net/wireless/mwlwifi/fwcmd.c > > create mode 100644 drivers/net/wireless/mwlwifi/fwcmd.h > > create mode 100644 drivers/net/wireless/mwlwifi/fwdl.c > > create mode 100644 drivers/net/wireless/mwlwifi/fwdl.h > > create mode 100644 drivers/net/wireless/mwlwifi/hostcmd.h > > create mode 100644 drivers/net/wireless/mwlwifi/isr.c > > create mode 100644 drivers/net/wireless/mwlwifi/isr.h > > create mode 100644 drivers/net/wireless/mwlwifi/mac80211.c > > create mode 100644 drivers/net/wireless/mwlwifi/mac80211.h > > create mode 100644 drivers/net/wireless/mwlwifi/main.c > > create mode 100644 drivers/net/wireless/mwlwifi/rx.c create mode > > 100644 drivers/net/wireless/mwlwifi/rx.h create mode 100644 > > drivers/net/wireless/mwlwifi/sysadpt.h > > create mode 100644 drivers/net/wireless/mwlwifi/tx.c create mode > > 100644 drivers/net/wireless/mwlwifi/tx.h > > > > diff --git a/drivers/net/wireless/Kconfig > > b/drivers/net/wireless/Kconfig index a63ab2e..1c60845 100644 > > --- a/drivers/net/wireless/Kconfig > > +++ b/drivers/net/wireless/Kconfig > > @@ -284,5 +284,6 @@ source "drivers/net/wireless/zd1211rw/Kconfig" > > source "drivers/net/wireless/mwifiex/Kconfig" > > source "drivers/net/wireless/cw1200/Kconfig" > > source "drivers/net/wireless/rsi/Kconfig" > > +source "drivers/net/wireless/mwlwifi/Kconfig" > > > > endif # WLAN > > diff --git a/drivers/net/wireless/Makefile > > b/drivers/net/wireless/Makefile index 6b9e729..1fe0f0d 100644 > > --- a/drivers/net/wireless/Makefile > > +++ b/drivers/net/wireless/Makefile > > @@ -62,3 +62,5 @@ obj-$(CONFIG_BRCMSMAC)+= brcm80211/ > > > > obj-$(CONFIG_CW1200) += cw1200/ > > obj-$(CONFIG_RSI_91X) += rsi/ > > + > > +obj-$(CONFIG_MWLWIFI) += mwlwifi/ > > diff --git a/drivers/net/wireless/mwlwifi/Kconfig > > b/drivers/net/wireless/mwlwifi/Kconfig > > new file mode 100644 > > index 000..3732223 > > --- /dev/null > > +++ b/drivers/net/wireless/mwlwifi/Kconfig > > @@ -0,0 +1,24 @@ > > +config MWLWIFI > > + tristate "Marvell Wireless Wi-Fi driver (mwlwifi)" > > Do you also have wired Wi-Fi cards? ;P > > The description seems very generic despite only supporting two chips. > Currently we already have two other marvell drivers claiming to support > "marvell" wireless: > > config MWL8K > tristate "Marvell 88W8xxx PCI/PCIe Wireless support" > > config MWIFIEX > tristate "Ma
RE: [PATCH v5] Add new mac80211 driver mwlwifi.
> Johannes Berg wrote:
>
> > +/* Bit definitio for MACREG_REG_A2H_INTERRUPT_CAUSE (A2HRIC) */
>
> typo
>
Will correct it.
>
> > +/* Bit definitio for MACREG_REG_H2A_INTERRUPT_CAUSE (H2ARIC) */
>
> same here
>
Will correct it.
>
> > +struct mwl_chip_info {
> > + char *part_name;
> > + char *fw_image;
> > +};
>
> const char *?
>
Will modify it.
>
> > +struct mwl_tx_hndl {
> > + struct sk_buff *psk_buff;
> > + u8 *sta_info;
>
> u8 * seems very strange for sta_info?
>
Will change it to "void *".
>
> > + struct mwl_tx_desc *pdesc;
> > + struct mwl_tx_hndl *pnext;
> > +};
>
> You probably shouldn't hand-write your own linked list implementation... You
> could possibly even put all this into the skb->cb and get rid of the extra
> struct
> entirely, just using an skb list?
>
> > +struct mwl_rx_hndl {
> > + struct sk_buff *psk_buff;/* associated sk_buff for Linux
> */
> > + struct mwl_rx_desc *pdesc;
> > + struct mwl_rx_hndl *pnext;
> > +};
>
> Ditto here.
>
These two structures are used to binding socket buffer and firmware descriptor.
It should be better to leave these fields here instead of moving them into
skb->cb.
>
> > + struct mwl_tx_pwr_tbl tx_pwr_tbl[SYSADPT_MAX_NUM_CHANNELS];
> > + u32 cdd; /* 0: off, 1: on */
>
> bool?
>
Will modify it.
>
> > + spinlock_t sta_lock; /* for private sta info
> */
> > + struct list_head sta_list; /* List of stations
> */
>
> Could consider using the mac80211 station iteration
> ieee80211_iterate_stations_atomic().
>
It is better to keep private station information list in mwlwifi driver.
>
> > +struct beacon_info {
> > + bool valid;
> > + u16 cap_info;
> > + u8 b_rate_set[SYSADPT_MAX_DATA_RATES_G];
> > + u8 op_rate_set[SYSADPT_MAX_DATA_RATES_G];
> > + u16 ie_wmm_len; /* Keep WMM IE */
> > + u8 *ie_wmm_ptr;
> > + u16 ie_rsn_len; /* Keep WPA IE */
> > + u8 *ie_rsn_ptr;
> > + u16 ie_rsn48_len; /* Keep WPA2 IE */
> > + u8 *ie_rsn48_ptr;
> > + u16 ie_ht_len;/* Keep HT IE */
> > + u8 *ie_ht_ptr;
> > + u8 ie_list_ht[148];
> > + u16 ie_vht_len; /* Keep VHT IE */
> > + u8 *ie_vht_ptr;
> > + u8 ie_list_vht[24];
> > +};
>
> That struct is packed really inefficiently with pointers and u16s interleaved.
> Those u16s can also be u8s.
>
Will rearrange them.
>
> > +struct mwl_vif {
> > + struct list_head list;
>
> Could also consider using iterate_active_interfaces() and friends.
>
It is better to keep private interface information list in mwlwifi driver.
>
> > + struct ieee80211_vif *vif;
>
> and you should probably embed the struct into the vif->priv, then you don't
> need a vif pointer in it (use container_of)
>
Will modify it.
>
> > + u16 seqno; /* Non AMPDU sequence number assigned by
> driver. */
>
> That seems a bit strange - why not just leave it up to mac80211 then?
>
> > + /* Indicate if this is station mode */
> > + bool is_sta;
>
> use vif->type instead
>
Will modify it.
>
> > +struct mwl_amsdu_frag {
> > + struct sk_buff *skb;
> > + u8 pad;
> > + u8 *cur_pos;
> > + u8 num;
> > + u32 jiffies;
>
> unsigned long
>
> also struct ordering is really about as bad as it could be with lots of
> padding
>
Will modify it.
> > +struct mwl_sta {
> > + struct list_head list;
> > + struct ieee80211_sta *sta;
>
> same here - embed the struct into sta->priv and get rid of the sta pointer
>
Will modify it.
>
> > +/* Transmission information to transmit a socket buffer. */ struct
> > +mwl_tx_ctrl {
> > + void *sta;
> > + void *vif;
> > + void *k_conf;
>
> void pointers don't seem so great
> But perhaps you do want them to avoid mistakes, but you should add a
> comment then.
> If you're not sure what mistakes I'm talking about - consider an interface or
> station that's removed while the packet is pending TX.
>
When interface or station is removed, related pending socket buffers will be
removed.
>
> > +static inline struct mwl_vif *mwl_dev_get_vif(const struct
> ieee80211_vif *vif)
> > +{
> > + return (struct mwl_vif *)&vif->drv_priv; }
> > +
> > +static inline struct mwl_sta *mwl_dev_get_sta(const struct
> ieee80211_sta *sta)
> > +{
> > + return (struct mwl_sta *)&sta->drv_priv; }
>
> Oh wait, so you *do* embed the structs - you can use container_of() to go the
> other way then and get rid of the pointers.
>
Will modify it.
>
> > +static int mwl_fwcmd_wait_complete(struct mwl_priv *priv, unsigned
> short cmd)
> > +{
> > + unsigned int curr_iteration = MAX_WAIT_FW_COMPLETE_ITERATIONS;
> > +
> > + unsigned short int_code = 0;
> > +
> > + do {
> > + int_code = le16_to_cpu(*((__le16 *)&priv
> ->pcmd_buf[0]));
> > + mdelay(1);
> > + } while ((int_code != cmd) && (--curr_iteration));
> > +
> > + if (curr_iteration == 0) {
> > + wiphy_err(priv->hw->wiphy, "cmd 0x%04x=%s timed
> out\n",
> > +
RE: [PATCH] Add FW binary used by mwlwifi driver.
Thanks. I do it now.
> -Original Message-
> From: Julian Calaby [mailto:julian.cal...@gmail.com]
> Sent: Saturday, August 22, 2015 11:00 AM
> To: David Lin
> Cc: dw...@infradead.org; b...@decadent.org.uk;
> linux-wireless@vger.kernel.org; Chor Teck Law; Pete Hsieh
> Subject: Re: [PATCH] Add FW binary used by mwlwifi driver.
>
> Hi David,
>
> On Sat, Aug 22, 2015 at 12:52 PM, David Lin wrote:
> > License is the same as previous Marvell's FW binary.
>
> You _MUST_ update the WHENCE file. Read the README I linked before.
>
> Thanks,
>
> --
> Julian Calaby
>
> Email: julian.cal...@gmail.com
> Profile: http://www.google.com/profiles/julian.calaby/
N�r��yb�X��ǧv�^�){.n�+{��*ޕ�,�{ay� ʇڙ�,j��f���h���z� �w���
���j:+v���w�j�mzZ+�ݢj"��!�i
RE: [PATCH v6] Add new mac80211 driver mwlwifi.
> Kalle Valo writes:
>
> David Lin writes:
>
> > The Linux driver for WRT1900AC. The work was initially developed as
> > part of openwrt effort and maintained on https://github.com/kaloz/mwlwifi.
> >
> > This is still work in progress, with 8864 chipset more mature and
> > tested, while 8897 for the similar use case is added recently.
> >
> > Signed-off-by: David Lin
>
> [...]
>
> > +static void mwl_reg_notifier(struct wiphy *wiphy,
> > +struct regulatory_request *request) { #ifdef
> > CONFIG_OF
> > + struct ieee80211_hw *hw;
> > + struct mwl_priv *priv;
> > + struct property *prop;
> > + struct property *fcc_prop = NULL;
> > + struct property *etsi_prop = NULL;
> > + struct property *specific_prop = NULL;
> > + u32 prop_value;
> > + int i, j, k;
> > +
> > + hw = (struct ieee80211_hw *)wiphy_priv(wiphy);
> > + priv = hw->priv;
> > +
> > + if (priv->pwr_node) {
> > + for_each_property_of_node(priv->pwr_node, prop) {
> > + if (strcmp(prop->name, "FCC") == 0)
> > + fcc_prop = prop;
> > + if (strcmp(prop->name, "ETSI") == 0)
> > + etsi_prop = prop;
> > + if ((prop->name[0] == request->alpha2[0]) &&
> > + (prop->name[1] == request->alpha2[1]))
> > + specific_prop = prop;
> > + }
> > +
> > + prop = NULL;
> > +
> > + if (specific_prop) {
> > + prop = specific_prop;
> > + } else {
> > + if (request->dfs_region == NL80211_DFS_ETSI)
> > + prop = etsi_prop;
> > + else
> > + prop = fcc_prop;
> > + }
> > +
> > + if (prop) {
> > + /* Reset the whole table */
> > + for (i = 0; i < SYSADPT_MAX_NUM_CHANNELS; i++)
> > + memset(&priv->tx_pwr_tbl[i], 0,
> > + sizeof(struct mwl_tx_pwr_tbl));
> > +
> > + /* Load related power table */
> > + i = 0;
> > + j = 0;
> > + while (i < prop->length) {
> > + prop_value =
> > + be32_to_cpu(*(__be32 *)
> > + (prop->value + i));
> > + priv->tx_pwr_tbl[j].channel = prop_value;
> > + i += 4;
> > + prop_value =
> > + be32_to_cpu(*(__be32 *)
> > + (prop->value + i));
> > + priv->tx_pwr_tbl[j].setcap = prop_value;
> > + i += 4;
> > + for (k = 0; k < SYSADPT_TX_POWER_LEVEL_TOTAL;
> > +k++) {
> > + prop_value =
> > + be32_to_cpu(*(__be32 *)
> > + (prop->value + i));
> > + priv->tx_pwr_tbl[j].tx_power[k] =
> > + prop_value;
> > + i += 4;
> > + }
> > + prop_value =
> > + be32_to_cpu(*(__be32 *)
> > + (prop->value + i));
> > + priv->tx_pwr_tbl[j].cdd =
> > + (prop_value == 0) ? false : true;
> > + i += 4;
> > + prop_value =
> > + be32_to_cpu(*(__be32 *)
> > + (prop->value + i));
> > + priv->tx_pwr_tbl[j].txantenna2 = prop_value;
> > + i += 4;
> > + j++;
> > + }
> > +
> > + /* Dump loaded power tabel */
> > + wiphy_info(hw->wiphy, "%s: %s\n", dev_name(&wiphy->dev),
> > +
RE: [PATCH v6] Add new mac80211 driver mwlwifi.
> Kalle Valo [mailto:kv...@codeaurora.org] writes: > > David Lin writes: > > > The Linux driver for WRT1900AC. The work was initially developed as > > part of openwrt effort and maintained on https://github.com/kaloz/mwlwifi. > > > > This is still work in progress, with 8864 chipset more mature and > > tested, while 8897 for the similar use case is added recently. > > > > Signed-off-by: David Lin > > The commit log is really short. For a new driver you should explain a lot > more, > for example what works and what doesn't. Also it should contained detailed > info about how and why (!) this conflicts with mwifiex and the way that > problem was solved. > > Also no new BUG_ON() calls in drivers, please: > > fwcmd.c:BUG_ON(tid >= SYSADPT_MAX_TID); > mac80211.c: BUG_ON(queue > SYSADPT_TX_WMM_QUEUES - 1); > mac80211.c: BUG_ON(stream->state != > AMPDU_STREAM_IN_PROGRESS); > tx.c: BUG_ON(tid > 7); > tx.c: BUG_ON(tid >= SYSADPT_MAX_TID); > tx.c: BUG_ON(!tx_skb); > > We should start to get rid of all 200+ calls we already have, wireless drivers > should never call BUG_ON(). Instead use descriptive error message > (WARN_ON() etc) and bail out nicely with an error. That's much more user > friendly than crashing the kernel. > > -- > Kalle Valo I will change BUG_ON() to WARN_ON() for next patch. Thanks, David -- 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 v5] Add new mac80211 driver mwlwifi.
> > Kalle Valo writes: > > > Chor Teck Law writes: > > > >> But for different generation of future chipsets/new technology, when > >> the hardware no longer permit us to use same driver, we will have to > >> re-evaluate our options. We cannot guarantee something way out into > >> the future. > > > > Yeah, that's understandable. But the community really dislikes having > > duplicated code/functionality, so please keep this in mind. > > > > If you can submit the next version soon we could try to get this > > driver to 4.4. We have few more weeks. > > I forgot to ask about the firmware image for this driver. Is that available in > linux-firmware.git? > I had already sent firmware patch for linux-firmware.git. However, it looks like it had not yet been added to linux-firmware.git. After sending out Patch v7, I will check to see if this patch can be added to linux-firmware.git. > -- > Kalle Valo Thanks, David From: David Lin Subject: [PATCH] Add FW binary used by mwlwifi driver. Newsgroups: gmane.linux.kernel.wireless.general Date: 2015-08-22 03:54:38 GMT (5 weeks, 3 days, 22 hours and 7 minutes ago) Signed-off-by: David Lin public.gmane.org> --- WHENCE | 12 mwlwifi/88W8864.bin | Bin 0 -> 116356 bytes mwlwifi/88W8897.bin | Bin 0 -> 489084 bytes 3 files changed, 12 insertions(+) mode change 100644 => 100755 WHENCE create mode 100755 mwlwifi/88W8864.bin create mode 100755 mwlwifi/88W8897.bin -- 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 v6] Add new mac80211 driver mwlwifi.
> Kalle Valo [mailto:kv...@codeaurora.org] writes:
>
> David Lin writes:
>
> > The Linux driver for WRT1900AC. The work was initially developed as
> > part of openwrt effort and maintained on https://github.com/kaloz/mwlwifi.
> >
> > This is still work in progress, with 8864 chipset more mature and
> > tested, while 8897 for the similar use case is added recently.
> >
> > Signed-off-by: David Lin
>
> [...]
>
> > +static void mwl_reg_notifier(struct wiphy *wiphy,
> > +struct regulatory_request *request) { #ifdef
> > CONFIG_OF
> > + struct ieee80211_hw *hw;
> > + struct mwl_priv *priv;
> > + struct property *prop;
> > + struct property *fcc_prop = NULL;
> > + struct property *etsi_prop = NULL;
> > + struct property *specific_prop = NULL;
> > + u32 prop_value;
> > + int i, j, k;
> > +
> > + hw = (struct ieee80211_hw *)wiphy_priv(wiphy);
> > + priv = hw->priv;
> > +
> > + if (priv->pwr_node) {
> > + for_each_property_of_node(priv->pwr_node, prop) {
> > + if (strcmp(prop->name, "FCC") == 0)
> > + fcc_prop = prop;
> > + if (strcmp(prop->name, "ETSI") == 0)
> > + etsi_prop = prop;
> > + if ((prop->name[0] == request->alpha2[0]) &&
> > + (prop->name[1] == request->alpha2[1]))
> > + specific_prop = prop;
> > + }
> > +
> > + prop = NULL;
> > +
> > + if (specific_prop) {
> > + prop = specific_prop;
> > + } else {
> > + if (request->dfs_region == NL80211_DFS_ETSI)
> > + prop = etsi_prop;
> > + else
> > + prop = fcc_prop;
> > + }
> > +
> > + if (prop) {
> > + /* Reset the whole table */
> > + for (i = 0; i < SYSADPT_MAX_NUM_CHANNELS; i++)
> > + memset(&priv->tx_pwr_tbl[i], 0,
> > + sizeof(struct mwl_tx_pwr_tbl));
> > +
> > + /* Load related power table */
> > + i = 0;
> > + j = 0;
> > + while (i < prop->length) {
> > + prop_value =
> > + be32_to_cpu(*(__be32 *)
> > + (prop->value + i));
> > + priv->tx_pwr_tbl[j].channel = prop_value;
> > + i += 4;
> > + prop_value =
> > + be32_to_cpu(*(__be32 *)
> > + (prop->value + i));
> > + priv->tx_pwr_tbl[j].setcap = prop_value;
> > + i += 4;
> > + for (k = 0; k < SYSADPT_TX_POWER_LEVEL_TOTAL;
> > +k++) {
> > + prop_value =
> > + be32_to_cpu(*(__be32 *)
> > + (prop->value + i));
> > + priv->tx_pwr_tbl[j].tx_power[k] =
> > + prop_value;
> > + i += 4;
> > + }
> > + prop_value =
> > + be32_to_cpu(*(__be32 *)
> > + (prop->value + i));
> > + priv->tx_pwr_tbl[j].cdd =
> > + (prop_value == 0) ? false : true;
> > + i += 4;
> > + prop_value =
> > + be32_to_cpu(*(__be32 *)
> > + (prop->value + i));
> > + priv->tx_pwr_tbl[j].txantenna2 = prop_value;
> > + i += 4;
> > + j++;
> > + }
> > +
> > + /* Dump loaded power tabel */
> > + wiphy_info(hw->wiphy, "%s: %s\n", dev_name(&wiphy->dev),
> > +
[PATCH v9] Add new mac80211 driver mwlwifi.
PATCH v8 changes since PATCH v7: - Used scnprintf() to replace sprintf() for debugfs output messages to avoid overwriting buffer boundary. - Used mutex to replace spinlock for the protection of firmware command. - Used NL80211_BAND_ instead of IEEE80211_BAND_ (in order to work with updated mac80211). - Used usleep_range() instead of mdelay(). - Modified the code to work with new mac80211 API ampdu_action() and get peer AMSDU information from parameters of this function instead of peeking ADDBA related packets. - Removed BA stream if traffic is not heavy. - Removed version information. - Added DFS, WPS, WDS and thermal function. - Changed length of mac vht_mpdu from 7991 to 3895. PATCH v9 changes since PATCH v8: - Added code to support Marvell WiFi chip with device power table. - Used IS_ENABLED macro to test for Kconfig symbols. David Lin (1): Add new mac80211 driver mwlwifi. MAINTAINERS |6 + drivers/net/wireless/marvell/Kconfig|1 + drivers/net/wireless/marvell/Makefile |1 + drivers/net/wireless/marvell/mwlwifi/Kconfig| 23 + drivers/net/wireless/marvell/mwlwifi/Makefile | 13 + drivers/net/wireless/marvell/mwlwifi/debugfs.c | 830 +++ drivers/net/wireless/marvell/mwlwifi/debugfs.h | 24 + drivers/net/wireless/marvell/mwlwifi/dev.h | 516 + drivers/net/wireless/marvell/mwlwifi/fwcmd.c| 2837 +++ drivers/net/wireless/marvell/mwlwifi/fwcmd.h| 223 ++ drivers/net/wireless/marvell/mwlwifi/fwdl.c | 186 ++ drivers/net/wireless/marvell/mwlwifi/fwdl.h | 25 + drivers/net/wireless/marvell/mwlwifi/hostcmd.h | 913 drivers/net/wireless/marvell/mwlwifi/isr.c | 172 ++ drivers/net/wireless/marvell/mwlwifi/isr.h | 27 + drivers/net/wireless/marvell/mwlwifi/mac80211.c | 719 ++ drivers/net/wireless/marvell/mwlwifi/main.c | 840 +++ drivers/net/wireless/marvell/mwlwifi/rx.c | 513 drivers/net/wireless/marvell/mwlwifi/rx.h | 25 + drivers/net/wireless/marvell/mwlwifi/sysadpt.h | 83 + drivers/net/wireless/marvell/mwlwifi/thermal.c | 182 ++ drivers/net/wireless/marvell/mwlwifi/thermal.h | 42 + drivers/net/wireless/marvell/mwlwifi/tx.c | 1250 ++ drivers/net/wireless/marvell/mwlwifi/tx.h | 37 + 24 files changed, 9488 insertions(+) create mode 100644 drivers/net/wireless/marvell/mwlwifi/Kconfig create mode 100644 drivers/net/wireless/marvell/mwlwifi/Makefile create mode 100644 drivers/net/wireless/marvell/mwlwifi/debugfs.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/debugfs.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/dev.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/fwcmd.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/fwcmd.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/fwdl.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/fwdl.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/hostcmd.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/isr.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/isr.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/mac80211.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/main.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/rx.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/rx.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/sysadpt.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/thermal.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/thermal.h create mode 100644 drivers/net/wireless/marvell/mwlwifi/tx.c create mode 100644 drivers/net/wireless/marvell/mwlwifi/tx.h -- 1.9.3
[PATCH] linux-firmware: mwlwifi: update 88W8864 firmware.
Release Version: 7.2.9.26.
Signed-off-by: David Lin
---
mwlwifi/88W8864.bin | Bin 116356 -> 118776 bytes
1 file changed, 0 insertions(+), 0 deletions(-)
diff --git a/mwlwifi/88W8864.bin b/mwlwifi/88W8864.bin
index
19b21a6bfbdd04073ebd62561fa8c2df8c80575b..e6d12b9efd98593578f3e49f0b1c91ba400657e8
100644
GIT binary patch
delta 52963
zcmZ5|4O~-I|M)p~uZ*`LsHhX~hTt&K;Y*@f4y4P5CS+Pxnis_yz7ER--!_yK6b-#r
z*i%oltgNgUYDuQg^vTkvmuFKcQOk>>plDqL-Gz=a^!tDIxx43{dtSfi`~9Bp
z+qo6n6yiIInrgXW6Grw;J9d7h`sd9dN7aYaN0SPwWh|mPw}VFYNl=7`oT|KG`IJ
z=jz`dF)83)`TN7Bb#Slvoy(IwV$=GMwyBK@;j<%}L-|5MR=!|_r{B7R-YD#E@=F6VGLZy0+evQM=IER3
zDl7|1g|$Js(B-3qBzXS0H_lXU$?9?xma|w((Q#?Qh2YUPjS*`lbewSRU+P0a@9)2N
zd0jn-Rv(?#ijmuZTz9>WV$LAf$XAbubgkXW&5)N;CvG43L2kv;kGtz0;*82O)R|)k
z5+X9ObXx6_U$_EQK~l8~BeWSKqZ1?Lrx@w~g;CTu7*!vK>%@U$su@^UeFCGe{|)8r
zsD_k>{$V7s0@X{vA{B~{a-*YCmG`=pM<=XX&vIETH09SDF>1v>;)U=1LH&>7a3uGfGD)rZU3O!faK?9ydnfvF
zO@#2Fh?!>?8b)RF?Ke%!M5SscdHT-BMa7JKP$BFV=iNjql{<@hQ;bF`;jq8fOj61W
z
z*2BFXo2u*M57L=11=E@gm_nqVW@3fH9y0fD;6Gki=Hq^gO_KH(p1HA8osimJKsR6+
zS?Ne~sFlZhq|6`@q0%F(%cy9tQqx{w3e)sQz9uHPW97A^-|
z>bb~;`7AND2GbW?({Bj*nEv#xWSzYxPU!5GGI7G-p7NTkJGwq}B}%z03I}lN2IjJO
zi-T*UFW=P}gUSciWUq3svR(2{HQq_?QT63$he7ipB}EU(ovLGR99x(exOW|{lAg{?9Q$cWkNR|i4e
z>T->~#64JVbq~>B^{`b3#i$yb+FT{oTRag?S=xHfNNGyvRpXu5K8`!Fab!fAO9lFs
z?z+3L*j{uy?%J)#
z`WOp48v<9!FA2)MvEjEgi~Dlg3vNEGc=U)KU<>F{9I_!qAyii5`e_$*jP;A%n_D}s
zlArP3?G=0O-L~$?p6{M`>1rD5-gDPlcv-C2@au$U-8k(T=(5%$*S&3$b||rw@4h;-
zS-e`_oOY$3@bMpcUC8)`nYO=Sx@JcfgHu^aH?U>9^5W+_tBIfK5h*&Nk}XpTwhw!#
zt<2-__atf3Knx@#<>R>7?AWa^cEGIe+Ks4F2n@!u{Xn
zwm3l^(O^@ak2ETtP@Iqa-*16p0M=k#L~6veh~tWvBhE)2{y*=-CAUSWq~{~G(zhaF
zq)7F@+UFyaRjI0#s#4Wj)h@`dQSMO<`CIGfBTuU2kr9!)$Z?SqBW;hstL(_S$b*r`
zBI&9aI^T?iwuUe+KJ349Ra-U96ycOx@3zXUF~ZhhN2|7&T^A!f=r3GH7c1IvT;;-M
z3(1)PIlP7BoLWd%*%ivh!m4mSyRNW`FsrQR=_;eb-Kbqh^2hkubz0jx@&Z5qZR_|8
zZ?7jvaXudY04KJUu79yfq?OYdW0&1<8d8{Gt^a5797PKT^ex=ip|=Y0
zJJDazR|s|w=ex!aG#z8r!q?rTmB$$Rek)B-)(CY@!yVDK7~y??BQ#}%CQrg0(a}On
zu&^pJFVoUj_@%p|wXo_bu`5=Q*H>_NA7f=EwZL_kwIZ}`3k(;SN*mk>(>b4kT#1R1
zt|+*#1;#S;1~4AjV4sF`DO|JRa#*ll=4leo^)WHpE<2uJyGrS|-p|N_jEt*6;MA-$
zM)v?x|Iv9wOb
zrfQ+On$bdKwUnQC(_DfZp%D9z2fuuk;KaGUVVu}ycSy8rj(48G--&`coI0(b+~0|}
z3NR1<&|ReZcxOc%PLp^j`nwwwWidRJCiCQqZS&T#N7=T?a$c53dA<|d5UF8nV;!q=
z#Y?gEu&xKhI*`g%1XO}r?6F`5f!M_NeqxFc2KH8pn2i=5_5+L%*^eIo{Cz09+&`z4
ztSRN;Er%DZzK-;MPs8G7cI9
zL+Jid&qZ+(n#V*4=l$a1%c8pexCN&z^qdk=!G_1Oi_AB(96
zuv-hM;p~Y{92YIjftHYspV0hQb8&i0VP_1jXP;&)7Gsx{wy-fYYKGacm&Hlt-@~j>
zN?}X_A~@mhAI1sRl633{bDQ7DNZMs_v&C^JuNY>HOprQV{Ga-AMJ&u)4KOnJ$=h6p
z%$~fxKmWen|Kwc&CMvCr%wPTAdg&;b|FA*D&nA76J)&v-&KEjmYG6X4N)*5Z>7s>U
z-O!kt3>}(MlcsP)l|tq@4>SbupkUCnbdFt`VofLcE7#neE>|+Hpqu-30zjVUV@7-DJzsE63|OWLoPHYsPx(^G;RcDdXG=T`)8KZ;x
zG(l3-E>mwP&TEh?!tEzfwq-$Qe0BuiWxG26%6HA-EM1pXN8LrEvsv=IRFh@#nT~f#
zvM5Vj4rXUF(2QX~@kiN{??rxo*Q~S{lVrT&%H1m+S3Td3|B~Ru4+FwD@nMPPYV{T2
z>Ygh~mh2kWy8i%5g5gTw>f`E^YZ0v!7WPO}Yzk%qqh}IAtWuFKHbsD@am^%WuL0GM
z_r>w$SGM!>m|`cD772tp$tNy$0lga;?2!Qjma`X3GB)5%3tF$SWkB;77`N{i6LTf|
z3SG6YL9QctiA+DCrd{0nsp!X<)t`z1Y}7M3aa*P6XoAHS-(vWI_?@4VW1b&!bFSoZ*<2C-a8>LMAeM1DXI
z|4Rc?TO|6k9z=SV#4%1&4Co_SEqg4dM7{dq7Kf$4Nijq%R12EkQ~kuDEO}*A8(K)#
z@ZMA6(1PLIph3yTL?~QXhV);GUtoC;zK7k0DTuF|@*zCi>!V@sQM8zNwv1tncP967
zwcF!xJ0LztWX7C+g1ntuKPXWVE$DjKvd!E_gS6@UK!Q(nvt@GKpM}c@Y^YVfyAKpc
zgiZ|#FW?iq-+iv1aGuK@IOM_FD@2&1?bqxM%W=g3_cB<@XyHXaTc*}4&fITBo0X#w
z8ha%?Q6gf5;oV%OCP`0svt{^x*4}On{|qb^*)f;HaMXIq8KjG26hd@2w_c;k-XI3N
zJA+|qFe;g%bpXk?yAy;lJ->)^FxF$Y4EF9E(<|1K)*O?}8qH^c@-MSyS!LRjR$0mM
z5>C)06j}X9t7M{thQK%V81O7gI2eG1(4MSUL++1(h>l9iRUc=O$fw*y#!*v^H)V8|
zU@fGlBo@vtjAQYY-e$*|d_hhHp}PT$v?B%#0jW3Rc}Rs&iN47_KRC%I(Kc_8c4*KC
z^u
zXd+xDU=sWOs|rakgsIE{h28>}BU7=0tj;%s!>f~5xOOBOR)_2ZBPd0;372YDe5}RR
zUs?+nNjvluy+|z_?dh6|=&r{idG5Et6S+A_`iJK=(^dKRf+ZL7E=-Nk?>ck24GFS7
za@9zgOlCmUt!!0_p^D&>*H9YI%8I{c6hGL-pd_NN}eF0GZ!NMk%ixyVuhvt
zqE>Y&;uA|LJw97lKDX7VVauO^(&~nP!7~4J?U=d!DrxgKUp|*HWSA|qw|6_Q#W_J|
z0+B$w1*hONr5mNnK}5hFltS@14LE;_6gV{oP7P}gx{RI3MX=}7RZ*Y~$vl_Fqt}1A
zLB&tBsCXg`1-$uz^SlJ8@Bt=|$zss!(vChTO86*1wPL*rXp=mhwFSm7!x=MU1T5Gd
zkOA3B?!P)5sDcS~`6@x6IMCg89dK6v0H__g?y9F^O98}eXwovFI!Jg$4vGWgLH}-)g0;gLS^J|%+HOR7h8Q4`r5bg5=f!#apIztv
zt?LVITs1v1{&}%N)S}0MJS)JAGL8?n=5s&Nqd2wp!N_su^{ruG&E_80#@oJYa1^im
z=->5-W7oMlPe>eFT%8qth|IdjPh{3V&aP~U78VDIG#dfZEz-BuQ*MQD-H&;N;0=}O
zJiDM}9Um>s4$2uQk+Mzf0X?v-e*H&a&bcr7&VW`-q4)F=lp=w*3Zb-j=@ZaJ_t~75
zOFYjLLxBMd1v_5);sXNgueD)dfAQp)>2}DEs{tbcTxcgo{vBb8c?+nc2(BnIg>s<&
zHz4bCDxJob>2ztN9SL}wxDH=gtKhq?@@u3WYV6X4a@Ncq+!wS{ltRWi55_s@{x~N`ThEp(M0bXKx0D1I04$AwVzQ2jfION2hMi9)Tu0why{&8Jeqvyl+*Ppuu&4x?hs^36lryd|Etp>Z+jV+%
zVOcOQTh!Xn7uSqo3y3A}%KvO60dKaSuQE9?>?0I3_}r&^EGIC40cBoyR2lV?@6Cc7
zM9X;PX=%HiBKk{w;k#
RE: [PATCH v9] Add new mac80211 driver mwlwifi.
> Johannes Berg [mailto:johan...@sipsolutions.net] wrote: > > On Wed, 2016-12-21 at 04:11 +0000, David Lin wrote: > > This patch provides the mwlwifi driver for Marvell 8863, 8864 and > > 8897 > > chipsets. > > This driver was developed as part of the openwrt.org project to > > support Linksys WRT1900AC and is maintained on > > https://github.com/kaloz/mwlwi fi. > > I found some minor things: > > * using kmalloc/memset instead of kzalloc I will change it. > * mwl_fwcmd_get_80m_pri_chnl() could use some arithmetic or at least >switch case consolidation - if it shouldn't even go away entirely >since mac80211 doesn't really limit you to the primary channel as >the spec requires it - so the whole purpose of the function seems a >bit questionable This function is used for the host command of firmware. > * wiphy_err(hw->wiphy, "failed execution\n"); seems like a bit too >generic (you have that many times - how are you going to tell which >happened?) I will modify it. > * rates = sta->supp_rates[NL80211_BAND_5GHZ] << 5; >is a bit questionable - that 5 should probably be defined (also, >does that mean you support 1,2, 5.5, 11 and *22MBps*? curious, >almost nobody does that afaict) This is used to comply with the parameters of host command of firmware. > * a lot of initializations like > struct mwl_vif *mwl_vif; >[...] > mwl_vif = mwl_dev_get_vif(vif); > >could be rolled into a single line of code (which you sometimes do, >apparently always for "priv" but never for "mwl_vif" or "pcmd"?) I will modify it. > * "celcius" isn't really spelled that way - and you probably mean >"temperature" anyway since Celsius is a unit :) I will change it. > * typo: firware I will fix it. > * a bunch of your IE handling structs seem duplicate with ieee80211.h No, only pointer is defined for extracting content of beacon. This driver does not redefine IE structure if it is already defined in ieee80211.h. > * and partially wrong - something with a vendor OUI can't be RSN >(maybe you meant WPA? though it's not quite clear to me why you need >this at all!) Yes. It means WPA. This driver does not handle content of it. It only needs the IE for host command of firmware. > * +#ifdef CONFIG_OF >+#include >+#endif >I don't think you have to guard that include ... > * +#ifdef CONFIG_DEBUG_FS >+#include "debugfs.h" >+#endif >likewise > * git am also complained about a blank line at EOF > I will remove them and fix blank line. > > The only thing that really seems questionable to me is the whole beacon > parsing (and apparent rebuilding in firmware?). It's very odd that you could > do > that, with a softmac device where all the authentication and association is > handled by hostapd anyway, and you can't possibly pretend to handle > everything hostapd might throw at you - this will mean that you'll have > features hostapd and every other mac80211 supports that you will silently > drop afaict - which is rather unexpected. > > First, you're parsing the data obtained from hostapd, in > mwl_fwcmd_parse_beacon(), and then you send them all to the firmware in > mwl_fwcmd_set_ies(), but only the things you actually understood. New > higher-level features you don't understand, or vendor beacon IEs that aren't > WMM, would be completely dropped. > > I'm not very happy with that behaviour. > > Why does the firmware require this? Why not just pack all IEs into the > pcmd->ie_list_len_proprietary, and leave everything else 0? Or - if > perhaps firmware for some reason requires HT/VHT to be treated specially, > only parse out the ones that are really needed specially and leave everything > else in the ie_list_len_proprietary? > > Also, this is dropping all the encryption stuff - even those are extensible > btw, > and hostapd might do so. Having the firmware rebuild those out of out-of-band > information is very unexpected for a mac80211 driver. > This driver just extracts needed IEs which is used for the host command of firmware. I think we will not support other vendor IEs. And if needed, we can add them to pcmd->ie_list_proprietary. > johannes
RE: [PATCH v9] Add new mac80211 driver mwlwifi.
> Johannes Berg [mailto:johan...@sipsolutions.net] wrote: > On Tue, 2017-01-10 at 01:32 +, David Lin wrote: > > > > The only thing that really seems questionable to me is the whole > > > beacon parsing (and apparent rebuilding in firmware?). It's very odd > > > that you could do that, with a softmac device where all the > > > authentication and association is handled by hostapd anyway, and you > > > can't possibly pretend to handle everything hostapd might throw at > > > you - this will mean that you'll have features hostapd and every > > > other mac80211 supports that you will silently drop afaict - which > > > is rather unexpected. > > > > > > First, you're parsing the data obtained from hostapd, in > > > mwl_fwcmd_parse_beacon(), and then you send them all to the firmware > > > in mwl_fwcmd_set_ies(), but only the things you actually understood. > > > New higher-level features you don't understand, or vendor beacon IEs > > > that aren't WMM, would be completely dropped. > > > > > > I'm not very happy with that behaviour. > > > > > > Why does the firmware require this? Why not just pack all IEs into > > > the pcmd->ie_list_len_proprietary, and leave everything else 0? Or > > > - if perhaps firmware for some reason requires HT/VHT to be treated > > > specially, only parse out the ones that are really needed specially > > > and leave everything else in the ie_list_len_proprietary? > > > > > > Also, this is dropping all the encryption stuff - even those are > > > extensible btw, and hostapd might do so. Having the firmware rebuild > > > those out of out-of-band information is very unexpected for a > > > mac80211 driver. > > > > > > > This driver just extracts needed IEs which is used for the host > > command of firmware. I think we will not support other vendor IEs. > > And if needed, we can add them to pcmd->ie_list_proprietary. > > Sure, I see that you're doing this. It still makes no sense though, since all > management frames are handled by hostapd anyway. It would make some > sense if you actually were going to handle association in the firmware, but it > makes no sense here, as far as I can tell. > > I'm not sure how hard a line I should draw here, but I'll warn you now that I > won't take this into consideration when adding new features to mac80211, and > certainly Jouni won't take it into account when adding new features to > hostapd, > so that such new features will then SILENTLY and UNEXPECTEDLY (due to > mac80211) not work with your driver. > > I strongly advise you to reconsider this and try to pass through all the IEs > so > that newly added features that shouldn't require firmware interaction (since > hostapd is handling all the association handshake and IEs to advertise the > feature) will automatically and cleanly work. > > If you really can't do that, for some reason, then for my benefit as the > mac80211 maintainer and future maintainers of your driver, I'd like to have > documentation in the driver as to why the firmware needs the driver to split > up all those IEs and what it does with them. After all, a common-sense > analysis > would suggest that the firmware has no need for it, since all configuration > should come through other places and all IEs are just for going out on the > air. > - The objective is to use the same production firmware binary for both the open source and proprietary driver. Same interface is currently used by proprietary driver for historically reason, while the open source HAL is adapting to it for the existing shipping product. - We will make changes and clean things up in future. I will spend effort to continue its maintenance and clean-up. Thanks, David > johannes
