* Re: [RFC PATCH 1/2] rtl8xxxu: Add rate adaptive related data
From: Joe Perches @ 2019-05-09 9:29 UTC (permalink / raw)
To: Daniel Drake, Chris Chiu
Cc: jes.sorensen, Kalle Valo, David Miller, linux-wireless, netdev,
Linux Kernel, Linux Upstreaming Team, Larry Finger
In-Reply-To: <CAD8Lp45WmPz2c+OnszFyaRL=veF0avEffwv3muwXNoeLcE0fhw@mail.gmail.com>
On Thu, 2019-05-09 at 16:11 +0800, Daniel Drake wrote:
> On Fri, May 3, 2019 at 3:22 PM Chris Chiu <chiu@endlessm.com> wrote:
> > Add wireless mode, signal strength level, and rate table index
> > to tell the firmware that we need to adjust the tx rate bitmap
> > accordingly.
[]
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
[]
> > +/*mlme related.*/
> > +enum wireless_mode {
> > + WIRELESS_MODE_UNKNOWN = 0,
> > + //Sub-Element
>
> Run these patches through checkpatch.pl, it'll have some suggestions
> to bring the coding style in line, for example not using // style
> comments.
just fyi:
checkpatch ignores // comments since 2016
(new in 2019: unless you add --ignore=c99_comment_tolerance)
These are the relevant checkpatch commits:
In 2016, commit dadf680de3c2 ("checkpatch: allow c99 style // comments")
In 2019, commit 98005e8c743f ("checkpatch: allow reporting C99 style comments")
^ permalink raw reply
* Re: [PATCH 5/6] cfg80211: add support for SAE authentication offload
From: Stanley Hsu @ 2019-05-09 9:21 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Chi-Hsien Lin, linux-wireless@vger.kernel.org,
brcm80211-dev-list@broadcom.com, brcm80211-dev-list,
Arend van Spriel, Franky Lin, Hante Meuleman, Wright Feng,
Kalle Valo
In-Reply-To: <2FC4C72C-9E57-4195-A682-F7BAE7F3981E@holtmann.org>
On Fri, Jan 04, 2019 at 10:26:33AM +0100, Marcel Holtmann wrote:
> Hi,
>
> > Let drivers advertise support for station-mode SAE authentication
> > offload with a new NL80211_EXT_FEATURE_SAE_OFFLOAD flag.
> >
> > Signed-off-by: Chung-Hsien Hsu <stanley.hsu@cypress.com>
> > Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
> > ---
> > include/linux/ieee80211.h | 1 +
> > include/net/cfg80211.h | 5 +++++
> > include/uapi/linux/nl80211.h | 16 ++++++++++++++++
> > net/wireless/nl80211.c | 14 ++++++++++++++
> > 4 files changed, 36 insertions(+)
> >
> > diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
> > index 3b04e72315e1..37d3e655e547 100644
> > --- a/include/linux/ieee80211.h
> > +++ b/include/linux/ieee80211.h
> > @@ -2596,6 +2596,7 @@ enum ieee80211_key_len {
> > #define FILS_ERP_MAX_RRK_LEN64
> >
> > #define PMK_MAX_LEN64
> > +#define SAE_PASSWORD_MAX_LEN128
> >
> > /* Public action codes (IEEE Std 802.11-2016, 9.6.8.1, Table 9-307) */
> > enum ieee80211_pub_actioncode {
> > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> > index e0c41eb1c860..5809dac97b33 100644
> > --- a/include/net/cfg80211.h
> > +++ b/include/net/cfg80211.h
> > @@ -740,6 +740,9 @@ struct survey_info {
> > *CFG80211_MAX_WEP_KEYS WEP keys
> > * @wep_tx_key: key index (0..3) of the default TX static WEP key
> > * @psk: PSK (for devices supporting 4-way-handshake offload)
> > + * @sae_pwd: password for SAE authentication (for devices supporting SAE
> > + *offload)
> > + * @sae_pwd_len: length of SAE password (for devices supporting SAE offload)
> > */
> > struct cfg80211_crypto_settings {
> > u32 wpa_versions;
> > @@ -755,6 +758,8 @@ struct cfg80211_crypto_settings {
> > struct key_params *wep_keys;
> > int wep_tx_key;
> > const u8 *psk;
> > +const u8 *sae_pwd;
> > +u16 sae_pwd_len;
> > };
> >
> > /**
> > diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> > index 12762afb3a07..4840aaed39ba 100644
> > --- a/include/uapi/linux/nl80211.h
> > +++ b/include/uapi/linux/nl80211.h
> > @@ -235,6 +235,15 @@
> > */
> >
> > /**
> > + * DOC: SAE authentication offload
> > + *
> > + * By setting @NL80211_EXT_FEATURE_SAE_OFFLOAD flag drivers can indicate they
> > + * support offloading SAE authentication for WPA3-Personal networks. In
> > + * %NL80211_CMD_CONNECT the password for SAE should be specified using
> > + * %NL80211_ATTR_SAE_PASSWORD.
> > + */
> > +
> > +/**
> > * enum nl80211_commands - supported nl80211 commands
> > *
> > * @NL80211_CMD_UNSPEC: unspecified command to catch errors
> > @@ -2288,6 +2297,9 @@ enum nl80211_commands {
> > *
> > * @NL80211_ATTR_FTM_RESPONDER_STATS: Nested attribute with FTM responder
> > *statistics, see &enum nl80211_ftm_responder_stats.
> > + * @NL80211_ATTR_SAE_PASSWORD: attribute for passing SAE password material. It
> > + *is used with %NL80211_CMD_CONNECT to provide password for offloading
> > + *SAE authentication for WPA3-Personal networks.
> > *
> > * @NL80211_ATTR_TIMEOUT: Timeout for the given operation in milliseconds (u32),
> > *if the attribute is not given no timeout is requested. Note that 0 is an
> > @@ -2743,6 +2755,7 @@ enum nl80211_attrs {
> > NL80211_ATTR_FTM_RESPONDER,
> >
> > NL80211_ATTR_FTM_RESPONDER_STATS,
> > +NL80211_ATTR_SAE_PASSWORD,
> >
> > NL80211_ATTR_TIMEOUT,
>
> so you are breaking user-space API on purpose here even when there was a clear comment where to add new attributes:
>
> /* add attributes here, update the policy in nl80211.c */
Hi Marcel,
Thanks for pointing this out. It was a mistake caused by rebasing the
patch. Will fix it in V2.
>
> More importantly, does this actually need a new attribute and you can not utilize what has already been added for mesh? If this attribute is solely for offload cases, then it might be better named accordingly. Also I am curious on how mixed WPA1/WPA2/WPA3 network credentials are now provided to a CMD_CONNECT. So the CMD_CONNECT description might require an update as well.
This new attribute is used to pass the sae_password value, set in the
configuration file of wpa_supplicant, for offloading SAE authentication.
It seems that the existing attributes can not be utilized for the
purpose. Could you please point it out if you know the proper one? To
reflect the content of the attribute, NL80211_ATTR_SAE_PASSWORD should
be a proper name.
As for the mixed WPA/WPA2/WPA3 network credentials, no key materials
will be provided in a NL80211_CMD_CONNECT for non-offload cases. When
offload is considered, there is no conflict between WPA/WPA2 4-way
handshake offload and SAE authentication offload. For the WPA/WPA2
4-way handshake offload, the PSK is specified using NL80211_ATTR_PMK in
the NL80211_CMD_CONNECT. The corresponding description can be found in
the section "DOC: WPA/WPA2 EAPOL handshake offload". As for the SAE
authentication offload, the sae_password value is provided by
NL80211_ATTR_SAE_PASSWORD in NL80211_CMD_CONNECT. It is described in
the section "DOC: SAE authentication offload" proposed in this patch.
Regards,
Chung-Hsien
>
> Regards
>
> Marcel
>
This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.
^ permalink raw reply
* Re: [RFC PATCH 2/2] rtl8xxxu: Add watchdog to update rate mask by signal strength
From: Chris Chiu @ 2019-05-09 9:17 UTC (permalink / raw)
To: Daniel Drake
Cc: Jes Sorensen, Kalle Valo, David Miller, linux-wireless, netdev,
Linux Kernel, Linux Upstreaming Team, Larry Finger
In-Reply-To: <CAD8Lp47_-6d2wCAs5QbuR6Mw2w91TyJ9W3kFiJHH4F_6dXqnHg@mail.gmail.com>
On Thu, May 9, 2019 at 4:11 PM Daniel Drake <drake@endlessm.com> wrote:
>
> Hi Chris,
>
> Great work on finding this!
>
> On Fri, May 3, 2019 at 3:22 PM Chris Chiu <chiu@endlessm.com> wrote:
> > Introduce watchdog to monitor signal then update the rate mask
> > accordingly. The rate mask update logic comes from the rtlwifi
> > refresh_rate_adaptive_mask() from different chips.
>
> You should expand your commit message here to summarise the key points
> in the cover letter. Specifically that matching this aspect of the
> vendor driver results in a significant TX performance increase which
> was previously stuck at 1mbps.
>
> > ---
> > .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 8 +
> > .../realtek/rtl8xxxu/rtl8xxxu_8723b.c | 151 ++++++++++++++++++
> > .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 38 +++++
> > 3 files changed, 197 insertions(+)
> >
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > index 771f58aa7cae..f97271951053 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> > @@ -1239,6 +1239,11 @@ struct rtl8xxxu_rate_adaptive {
> > u8 rssi_level; /* INIT, HIGH, MIDDLE, LOW */
> > } __packed;
> >
> > +struct rtl8xxxu_watchdog {
> > + struct ieee80211_vif *vif;
> > + struct delayed_work ra_wq;
> > +};
>
> Having to store the vif address under the device-specific private
> structure may be a layering violation, but I'm not fully grasping how
> all this fits together. Can anyone from linux-wireless help?
>
> The existing rtl8xxxu_add_interface() code appears to allow multiple
> STA interfaces to be added. Does that imply that the hardware should
> support connecting to multiple APs on different channels? I'm pretty
> sure the hardware doesn't support that; if so we could do something
> similar to ar5523.c where it only allows a single vif, and can easily
> store that pointer in the device-specific structure.
>
> Or if there's a valid reason to support multiple vifs, then we need to
> figure out how to implement this watchdog. As shown below, the
> watchdog needs to know the supported rate info of the AP you are
> connected to, and the RSSI, and that comes from a specific vif. If
> multiple vifs are present, how would we know which one to choose for
> this rate adjustment?
>
I need the vif because there's seems no easy way to get RSSI. Please
suggest if there's any better idea for this. I believe multiple vifs is for AP
mode (with more than 1 virtual AP/SSIDs) and the Station+AP coexist
mode. But the rtl8xxxu driver basically supports only Station mode. So
the vif helps get the RSSI. Or I will need to record the RSSI in
rtl8xxxu_rx_parse_phystats() and store in rtl8xxxu_priv. That's a little
nasty. Maybe someone can point out how to retrieve the RSSI from
specific register or which part of code in rtlwifi I can use to get the
undecorated signal strength.
> > struct rtl8xxxu_priv {
> > struct ieee80211_hw *hw;
> > struct usb_device *udev;
> > @@ -1344,6 +1349,7 @@ struct rtl8xxxu_priv {
> > u8 no_pape:1;
> > u8 int_buf[USB_INTR_CONTENT_LENGTH];
> > struct rtl8xxxu_rate_adaptive ra_info;
> > + struct rtl8xxxu_watchdog watchdog;
> > };
> >
> > struct rtl8xxxu_rx_urb {
> > @@ -1380,6 +1386,8 @@ struct rtl8xxxu_fileops {
> > bool ht40);
> > void (*update_rate_mask) (struct rtl8xxxu_priv *priv,
> > u32 ramask, int sgi);
> > + void (*refresh_rate_mask) (struct rtl8xxxu_priv *priv, int signal,
> > + struct ieee80211_sta *sta);
> > void (*report_connect) (struct rtl8xxxu_priv *priv,
> > u8 macid, bool connect);
> > void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > index 26b674aca125..92c35afecae0 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> > @@ -1645,6 +1645,156 @@ static void rtl8723bu_init_statistics(struct rtl8xxxu_priv *priv)
> > rtl8xxxu_write32(priv, REG_OFDM0_FA_RSTC, val32);
> > }
> >
> > +static u8 rtl8723b_signal_to_rssi(int signal)
> > +{
> > + if (signal < -95)
> > + signal = -95;
> > + return (u8)(signal + 95);
> > +}
> > +
> > +static void rtl8723b_refresh_rate_mask(struct rtl8xxxu_priv *priv,
> > + int signal, struct ieee80211_sta *sta)
> > +{
> > + struct rtl8xxxu_rate_adaptive *ra;
> > + struct ieee80211_hw *hw = priv->hw;
> > + u16 wireless_mode;
> > + u8 rssi_level, ratr_index;
> > + u8 txbw_40mhz;
> > + u8 rssi, rssi_thresh_high, rssi_thresh_low;
> > +
> > + ra = &priv->ra_info;
> > + wireless_mode = ra->wireless_mode;
> > + rssi_level = ra->rssi_level;
> > + rssi = rtl8723b_signal_to_rssi(signal);
> > + ratr_index = ra->ratr_index;
> > + txbw_40mhz = (hw->conf.chandef.width == NL80211_CHAN_WIDTH_40)? 1 : 0;
> > +
> > + switch (rssi_level) {
> > + case RTL8XXXU_RATR_STA_HIGH:
> > + rssi_thresh_high = 50;
> > + rssi_thresh_low = 20;
> > + break;
> > + case RTL8XXXU_RATR_STA_MID:
> > + rssi_thresh_high = 55;
> > + rssi_thresh_low = 20;
> > + break;
> > + case RTL8XXXU_RATR_STA_LOW:
> > + rssi_thresh_high = 60;
> > + rssi_thresh_low = 25;
> > + break;
> > + default:
> > + rssi_thresh_high = 50;
> > + rssi_thresh_low = 20;
> > + break;
> > + }
> > +
> > + if (rssi > rssi_thresh_high)
> > + rssi_level = RTL8XXXU_RATR_STA_HIGH;
> > + else if (rssi > rssi_thresh_low)
> > + rssi_level = RTL8XXXU_RATR_STA_MID;
> > + else
> > + rssi_level = RTL8XXXU_RATR_STA_LOW;
> > +
> > + if (rssi_level != ra->rssi_level) {
> > + int sgi = 0;
> > + u32 rate_bitmap = 0;
> > +
> > + rcu_read_lock();
> > + rate_bitmap = (sta->supp_rates[0] & 0xfff) |
> > + sta->ht_cap.mcs.rx_mask[0] << 12 |
> > + sta->ht_cap.mcs.rx_mask[1] << 20;
> > + if (sta->ht_cap.cap &
> > + (IEEE80211_HT_CAP_SGI_40 | IEEE80211_HT_CAP_SGI_20))
> > + sgi = 1;
> > + rcu_read_unlock();
> > +
> > + switch (wireless_mode) {
> > + case WIRELESS_MODE_B:
> > + ratr_index = RATEID_IDX_B;
> > + if (rate_bitmap & 0x0000000c)
> > + rate_bitmap &= 0x0000000d;
> > + else
> > + rate_bitmap &= 0x0000000f;
> > + break;
> > + case WIRELESS_MODE_A:
> > + case WIRELESS_MODE_G:
> > + ratr_index = RATEID_IDX_G;
> > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> > + rate_bitmap &= 0x00000f00;
> > + else
> > + rate_bitmap &= 0x00000ff0;
> > + break;
> > + case (WIRELESS_MODE_B|WIRELESS_MODE_G):
> > + ratr_index = RATEID_IDX_BG;
> > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> > + rate_bitmap &= 0x00000f00;
> > + else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> > + rate_bitmap &= 0x00000ff0;
> > + else
> > + rate_bitmap &= 0x00000ff5;
> > + break;
> > + case WIRELESS_MODE_N_24G:
> > + case WIRELESS_MODE_N_5G:
> > + case (WIRELESS_MODE_G|WIRELESS_MODE_N_24G):
> > + case (WIRELESS_MODE_A|WIRELESS_MODE_N_5G):
> > + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > + ratr_index = RATEID_IDX_GN_N2SS;
> > + else
> > + ratr_index = RATEID_IDX_GN_N1SS;
> > + case (WIRELESS_MODE_B|WIRELESS_MODE_G|WIRELESS_MODE_N_24G):
> > + case (WIRELESS_MODE_B|WIRELESS_MODE_N_24G):
> > + if (txbw_40mhz) {
> > + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > + ratr_index = RATEID_IDX_BGN_40M_2SS;
> > + else
> > + ratr_index = RATEID_IDX_BGN_40M_1SS;
> > + }
> > + else {
> > + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> > + ratr_index = RATEID_IDX_BGN_20M_2SS_BN;
> > + else
> > + ratr_index = RATEID_IDX_BGN_20M_1SS_BN;
> > + }
> > +
> > + if (priv->tx_paths == 2 && priv->rx_paths == 2) {
> > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> > + rate_bitmap &= 0x0f8f0000;
> > + else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> > + rate_bitmap &= 0x0f8ff000;
> > + else {
> > + if (txbw_40mhz)
> > + rate_bitmap &= 0x0f8ff015;
> > + else
> > + rate_bitmap &= 0x0f8ff005;
> > + }
> > + }
> > + else {
> > + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> > + rate_bitmap &= 0x000f0000;
> > + else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> > + rate_bitmap &= 0x000ff000;
> > + else {
> > + if (txbw_40mhz)
> > + rate_bitmap &= 0x000ff015;
> > + else
> > + rate_bitmap &= 0x000ff005;
> > + }
> > + }
> > + break;
> > + default:
> > + ratr_index = RATEID_IDX_BGN_40M_2SS;
> > + rate_bitmap &= 0x0fffffff;
> > + break;
> > + }
> > +
> > + ra->ratr_index = ratr_index;
> > + ra->rssi_level = rssi_level;
> > + priv->fops->update_rate_mask(priv, rate_bitmap, sgi);
> > + }
> > +
> > + return;
> > +}
> > +
> > struct rtl8xxxu_fileops rtl8723bu_fops = {
> > .parse_efuse = rtl8723bu_parse_efuse,
> > .load_firmware = rtl8723bu_load_firmware,
> > @@ -1665,6 +1815,7 @@ struct rtl8xxxu_fileops rtl8723bu_fops = {
> > .usb_quirks = rtl8xxxu_gen2_usb_quirks,
> > .set_tx_power = rtl8723b_set_tx_power,
> > .update_rate_mask = rtl8xxxu_gen2_update_rate_mask,
> > + .refresh_rate_mask = rtl8723b_refresh_rate_mask,
> > .report_connect = rtl8xxxu_gen2_report_connect,
> > .fill_txdesc = rtl8xxxu_fill_txdesc_v2,
> > .writeN_block_size = 1024,
> > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > index 360e9bd837e5..8db479986e97 100644
> > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> > @@ -4565,6 +4565,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> > sgi = 1;
> > rcu_read_unlock();
> >
> > + priv->watchdog.vif = vif;
> > ra = &priv->ra_info;
> > ra->wireless_mode = rtl8xxxu_wireless_mode(hw, sta);
> > ra->ratr_index = RATEID_IDX_BGN_40M_2SS;
> > @@ -5822,6 +5823,38 @@ rtl8xxxu_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> > return 0;
> > }
> >
> > +static void rtl8xxxu_watchdog_callback(struct work_struct *work)
> > +{
> > + struct ieee80211_vif *vif;
> > + struct rtl8xxxu_watchdog *wdog;
> > + struct rtl8xxxu_priv *priv;
> > +
> > + wdog = container_of(work, struct rtl8xxxu_watchdog, ra_wq.work);
> > + priv = container_of(wdog, struct rtl8xxxu_priv, watchdog);
> > + vif = wdog->vif;
> > +
> > + if (vif) {
> > + int signal;
> > + struct ieee80211_sta *sta;
> > +
> > + rcu_read_lock();
>
> Can you explain the lock/unlock here?
>
Actually it may not be mandatory because the sta_info_get_bss()
will do this inside ieee80211_find_sta().
> > + sta = ieee80211_find_sta(vif, vif->bss_conf.bssid);
> > + if (!sta) {
> > + struct device *dev = &priv->udev->dev;
> > + dev_info(dev, "%s: no sta found\n", __func__);
>
> Does this result in a kernel log message every 2 seconds when the wifi
> interface is not associated to an AP?
Yes. It prints every 2 seconds if not associated.
>
> > + rcu_read_unlock();
> > + return;
> > + }
> > + rcu_read_unlock();
> > +
> > + signal = ieee80211_ave_rssi(vif);
> > + if (priv->fops->refresh_rate_mask)
> > + priv->fops->refresh_rate_mask(priv, signal, sta);
> > + }
> > +
> > + schedule_delayed_work(&priv->watchdog.ra_wq, 2 * HZ);
> > +}
> > +
> > static int rtl8xxxu_start(struct ieee80211_hw *hw)
> > {
> > struct rtl8xxxu_priv *priv = hw->priv;
> > @@ -5878,6 +5911,8 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
> >
> > ret = rtl8xxxu_submit_rx_urb(priv, rx_urb);
> > }
> > +
> > + schedule_delayed_work(&priv->watchdog.ra_wq, 2* HZ);
> > exit:
> > /*
> > * Accept all data and mgmt frames
> > @@ -6101,6 +6136,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> > INIT_LIST_HEAD(&priv->rx_urb_pending_list);
> > spin_lock_init(&priv->rx_urb_lock);
> > INIT_WORK(&priv->rx_urb_wq, rtl8xxxu_rx_urb_work);
> > + INIT_DELAYED_WORK(&priv->watchdog.ra_wq, rtl8xxxu_watchdog_callback);
> >
> > usb_set_intfdata(interface, hw);
> >
> > @@ -6226,6 +6262,8 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
> > mutex_destroy(&priv->usb_buf_mutex);
> > mutex_destroy(&priv->h2c_mutex);
> >
> > + cancel_delayed_work_sync(&priv->watchdog.ra_wq);
> > +
> > if (priv->udev->state != USB_STATE_NOTATTACHED) {
> > dev_info(&priv->udev->dev,
> > "Device still attached, trying to reset\n");
> > --
> > 2.21.0
> >
^ permalink raw reply
* Re: [PATCH 5/6] cfg80211: add support for SAE authentication offload
From: Stanley Hsu @ 2019-05-09 9:02 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Chi-Hsien Lin, linux-wireless@vger.kernel.org,
brcm80211-dev-list@broadcom.com, brcm80211-dev-list, Franky Lin,
Hante Meuleman, Wright Feng, Kalle Valo
In-Reply-To: <6c17fb65-d070-51d5-17cf-4d6788e8cbcb@broadcom.com>
On Fri, Jan 04, 2019 at 12:10:01PM +0100, Arend Van Spriel wrote:
> On 1/4/2019 7:11 AM, Chi-Hsien Lin wrote:
> >From: Chung-Hsien Hsu <stanley.hsu@cypress.com>
> >
> >Let drivers advertise support for station-mode SAE authentication
> >offload with a new NL80211_EXT_FEATURE_SAE_OFFLOAD flag.
>
> When touching nl80211 api I prefer 'nl80211:' prefix instead of 'cfg80211:'.
Thanks for the comment. Will change it in V2.
>
> >Signed-off-by: Chung-Hsien Hsu <stanley.hsu@cypress.com>
> >Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
> >---
> > include/linux/ieee80211.h | 1 +
> > include/net/cfg80211.h | 5 +++++
> > include/uapi/linux/nl80211.h | 16 ++++++++++++++++
> > net/wireless/nl80211.c | 14 ++++++++++++++
> > 4 files changed, 36 insertions(+)
> >
> >diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
> >index 3b04e72315e1..37d3e655e547 100644
> >--- a/include/linux/ieee80211.h
> >+++ b/include/linux/ieee80211.h
> >@@ -2596,6 +2596,7 @@ enum ieee80211_key_len {
> > #define FILS_ERP_MAX_RRK_LEN64
> > #define PMK_MAX_LEN64
> >+#define SAE_PASSWORD_MAX_LEN128
> > /* Public action codes (IEEE Std 802.11-2016, 9.6.8.1, Table 9-307) */
> > enum ieee80211_pub_actioncode {
> >diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> >index e0c41eb1c860..5809dac97b33 100644
> >--- a/include/net/cfg80211.h
> >+++ b/include/net/cfg80211.h
> >@@ -740,6 +740,9 @@ struct survey_info {
> > *CFG80211_MAX_WEP_KEYS WEP keys
> > * @wep_tx_key: key index (0..3) of the default TX static WEP key
> > * @psk: PSK (for devices supporting 4-way-handshake offload)
> >+ * @sae_pwd: password for SAE authentication (for devices supporting SAE
> >+ *offload)
> >+ * @sae_pwd_len: length of SAE password (for devices supporting SAE offload)
> > */
> > struct cfg80211_crypto_settings {
> > u32 wpa_versions;
> >@@ -755,6 +758,8 @@ struct cfg80211_crypto_settings {
> > struct key_params *wep_keys;
> > int wep_tx_key;
> > const u8 *psk;
> >+const u8 *sae_pwd;
> >+u16 sae_pwd_len;
>
> Not really need u16 as max length is defined earlier as 128.
It will be changed to u8 in V2.
>
> > };
> > /**
> >diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> >index 12762afb3a07..4840aaed39ba 100644
> >--- a/include/uapi/linux/nl80211.h
> >+++ b/include/uapi/linux/nl80211.h
> >@@ -235,6 +235,15 @@
> > */
> > /**
> >+ * DOC: SAE authentication offload
> >+ *
> >+ * By setting @NL80211_EXT_FEATURE_SAE_OFFLOAD flag drivers can indicate they
> >+ * support offloading SAE authentication for WPA3-Personal networks. In
> >+ * %NL80211_CMD_CONNECT the password for SAE should be specified using
> >+ * %NL80211_ATTR_SAE_PASSWORD.
> >+ */
> >+
> >+/**
> > * enum nl80211_commands - supported nl80211 commands
> > *
> > * @NL80211_CMD_UNSPEC: unspecified command to catch errors
> >@@ -2288,6 +2297,9 @@ enum nl80211_commands {
> > *
> > * @NL80211_ATTR_FTM_RESPONDER_STATS: Nested attribute with FTM responder
> > *statistics, see &enum nl80211_ftm_responder_stats.
> >+ * @NL80211_ATTR_SAE_PASSWORD: attribute for passing SAE password material. It
> >+ *is used with %NL80211_CMD_CONNECT to provide password for offloading
> >+ *SAE authentication for WPA3-Personal networks.
> > *
> > * @NL80211_ATTR_TIMEOUT: Timeout for the given operation in milliseconds (u32),
> > *if the attribute is not given no timeout is requested. Note that 0 is an
> >@@ -2743,6 +2755,7 @@ enum nl80211_attrs {
> > NL80211_ATTR_FTM_RESPONDER,
> > NL80211_ATTR_FTM_RESPONDER_STATS,
> >+NL80211_ATTR_SAE_PASSWORD,
>
> Marcel already commented on this. These enumerations are ABI. You
> should also change the nl80211_policy to assure the max length
> SAE_PASSWORD_MAX_LEN is not exceeded.
It will be moved to the bottom. The corresponding change in
nl80211_policy has been made.
Regards,
Chung-Hsien
>
> Gr. AvS
This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.
^ permalink raw reply
* Re: [PATCH 2/6] brcmfmac: send port authorized event for 802.1X 4-way handshake offload
From: Stanley Hsu @ 2019-05-09 8:58 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Chi-Hsien Lin, linux-wireless@vger.kernel.org,
brcm80211-dev-list@broadcom.com, brcm80211-dev-list, Franky Lin,
Hante Meuleman, Wright Feng, Kalle Valo
In-Reply-To: <d89b1304-471d-f064-02f1-6a9fab0ca25d@broadcom.com>
On Mon, Jan 07, 2019 at 10:44:01AM +0100, Arend Van Spriel wrote:
> On 1/4/2019 7:11 AM, Chi-Hsien Lin wrote:
> >From: Chung-Hsien Hsu <stanley.hsu@cypress.com>
> >
> >With 4-way handshake offload for 802.1X authentication, a port
> >authorized event should be sent to user space after the completion of
> >4-way handshake. It is used to indicate that a connection is authorized
> >and 802.1X authentication is no longer required.
>
> It had been a while since I had looked at our offload code
> (basically since the initial implementation for the nl80211 work) so
> I was unsure why this would be needed.
>
> So initially we added a PORT_AUTHORIZED *attribute* in the nl80211
> api and later on the PORT_AUTHORIZED *event* was introduced and
> 4-way hs offload support in wpa_supplicant is ignoring the
> *attribute* and only handling the *event*. I think this information
> is important enough to add to this commit message with a reference
> to commit 503c1fb98ba3 ("cfg80211/nl80211: add a port authorized
> event") which "broke" the functionality in brcmfmac.
Thanks a lot for the feedback.
After looking further, it is observed that the connection state will be
set to WPA_COMPLETED in wpa_supplicant after it sets PMK to the driver.
So no need to have this change. Let's drop it form the series.
Regards,
Chung-Hsien
This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.
^ permalink raw reply
* Re: [PATCH 1/6] nl80211: add NL80211_ATTR_IFINDEX to port authorized event
From: Stanley Hsu @ 2019-05-09 8:51 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Chi-Hsien Lin, linux-wireless@vger.kernel.org,
brcm80211-dev-list@broadcom.com, brcm80211-dev-list, Franky Lin,
Hante Meuleman, Wright Feng, Kalle Valo
In-Reply-To: <c8927d9e-09fc-e707-35b3-33a71966b29d@broadcom.com>
On Fri, Jan 04, 2019 at 11:51:19AM +0100, Arend Van Spriel wrote:
> On 1/4/2019 7:11 AM, Chi-Hsien Lin wrote:
> >From: Chung-Hsien Hsu <stanley.hsu@cypress.com>
> >
> >Add NL80211_ATTR_IFINDEX attribute to port authorized event to indicate
> >the operating interface of the device.
> >
> >Signed-off-by: Chung-Hsien Hsu <stanley.hsu@cypress.com>
> >Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
> >---
> > net/wireless/nl80211.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> >index 5e49492d5911..594aeba2982a 100644
> >--- a/net/wireless/nl80211.c
> >+++ b/net/wireless/nl80211.c
> >@@ -14727,7 +14727,8 @@ void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
> > return;
> > }
> >-if (nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid))
> >+if (nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex) ||
> >+ nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid))
> > goto nla_put_failure;
>
> Maybe also put NL80211_ATTR_WIPHY in the notification to be
> consistent with the other MLME notifications.
Thanks for the comment. Will include it in V2.
Regards,
Chung-Hsien
>
> Regards,
> Arend
This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.
^ permalink raw reply
* Re: [RFC PATCH 1/2] rtl8xxxu: Add rate adaptive related data
From: Daniel Drake @ 2019-05-09 8:11 UTC (permalink / raw)
To: Chris Chiu
Cc: jes.sorensen, Kalle Valo, David Miller, linux-wireless, netdev,
Linux Kernel, Linux Upstreaming Team, Larry Finger
In-Reply-To: <20190503072146.49999-2-chiu@endlessm.com>
Hi Chris,
Thanks for this! Some suggestions below, although let me know if any
don't make sense.
On Fri, May 3, 2019 at 3:22 PM Chris Chiu <chiu@endlessm.com> wrote:
>
> Add wireless mode, signal strength level, and rate table index
> to tell the firmware that we need to adjust the tx rate bitmap
> accordingly.
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 45 +++++++++++++++++++
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 45 ++++++++++++++++++-
> 2 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 8828baf26e7b..771f58aa7cae 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1195,6 +1195,50 @@ struct rtl8723bu_c2h {
>
> struct rtl8xxxu_fileops;
>
> +/*mlme related.*/
> +enum wireless_mode {
> + WIRELESS_MODE_UNKNOWN = 0,
> + //Sub-Element
Run these patches through checkpatch.pl, it'll have some suggestions
to bring the coding style in line, for example not using // style
comments.
> + WIRELESS_MODE_B = BIT(0), // tx: cck only , rx: cck only, hw: cck
> + WIRELESS_MODE_G = BIT(1), // tx: ofdm only, rx: ofdm & cck, hw: cck & ofdm
> + WIRELESS_MODE_A = BIT(2), // tx: ofdm only, rx: ofdm only, hw: ofdm only
> + WIRELESS_MODE_N_24G = BIT(3), // tx: MCS only, rx: MCS & cck, hw: MCS & cck
> + WIRELESS_MODE_N_5G = BIT(4), // tx: MCS only, rx: MCS & ofdm, hw: ofdm only
> + WIRELESS_AUTO = BIT(5),
> + WIRELESS_MODE_AC = BIT(6),
> + WIRELESS_MODE_MAX = (WIRELESS_MODE_B|WIRELESS_MODE_G|WIRELESS_MODE_A|WIRELESS_MODE_N_24G|WIRELESS_MODE_N_5G|WIRELESS_MODE_AC),
> +};
> +
> +/* from rtlwifi/wifi.h */
> +enum ratr_table_mode_new {
> + RATEID_IDX_BGN_40M_2SS = 0,
> + RATEID_IDX_BGN_40M_1SS = 1,
> + RATEID_IDX_BGN_20M_2SS_BN = 2,
> + RATEID_IDX_BGN_20M_1SS_BN = 3,
> + RATEID_IDX_GN_N2SS = 4,
> + RATEID_IDX_GN_N1SS = 5,
> + RATEID_IDX_BG = 6,
> + RATEID_IDX_G = 7,
> + RATEID_IDX_B = 8,
> + RATEID_IDX_VHT_2SS = 9,
> + RATEID_IDX_VHT_1SS = 10,
> + RATEID_IDX_MIX1 = 11,
> + RATEID_IDX_MIX2 = 12,
> + RATEID_IDX_VHT_3SS = 13,
> + RATEID_IDX_BGN_3SS = 14,
> +};
> +
> +#define RTL8XXXU_RATR_STA_INIT 0
> +#define RTL8XXXU_RATR_STA_HIGH 1
> +#define RTL8XXXU_RATR_STA_MID 2
> +#define RTL8XXXU_RATR_STA_LOW 3
> +
> +struct rtl8xxxu_rate_adaptive {
> + u16 wireless_mode;
> + u8 ratr_index;
> + u8 rssi_level; /* INIT, HIGH, MIDDLE, LOW */
> +} __packed;
It would be better/cleaner to avoid storing data in per-device
structures if at all possible.
For wireless_mode, I think you should just call
rtl8xxxu_wireless_mode() every time from rtl8723b_refresh_rate_mask().
The work done there is simple (i.e. it's not expensive to call) and
then we avoid having to store data (which might become stale etc).
For ratr_index, I believe you can just make it a parameter passed to
rtl8xxxu_gen2_update_rate_mask which is the only consumer of this
variable. The two callsites (rtl8xxxu_bss_info_changed and
rtl8723b_refresh_rate_mask) already know which value they want to be
used.
rssi_level is the one that you probably do want to store, since I see
the logic in patch 2 - if the rssi level hasn't changed then you don't
need to set the rate mask again, and that's a good idea since it
reduces bus traffic. You could move this into rtl8xxxu_priv rather
than having its own separate structure.
After making those changes it might even make sense to collapse this
all into a single patch; you can judge!
^ permalink raw reply
* Re: [RFC PATCH 2/2] rtl8xxxu: Add watchdog to update rate mask by signal strength
From: Daniel Drake @ 2019-05-09 8:11 UTC (permalink / raw)
To: Chris Chiu
Cc: jes.sorensen, Kalle Valo, David Miller, linux-wireless, netdev,
Linux Kernel, Linux Upstreaming Team, Larry Finger
In-Reply-To: <20190503072146.49999-3-chiu@endlessm.com>
Hi Chris,
Great work on finding this!
On Fri, May 3, 2019 at 3:22 PM Chris Chiu <chiu@endlessm.com> wrote:
> Introduce watchdog to monitor signal then update the rate mask
> accordingly. The rate mask update logic comes from the rtlwifi
> refresh_rate_adaptive_mask() from different chips.
You should expand your commit message here to summarise the key points
in the cover letter. Specifically that matching this aspect of the
vendor driver results in a significant TX performance increase which
was previously stuck at 1mbps.
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 8 +
> .../realtek/rtl8xxxu/rtl8xxxu_8723b.c | 151 ++++++++++++++++++
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 38 +++++
> 3 files changed, 197 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 771f58aa7cae..f97271951053 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1239,6 +1239,11 @@ struct rtl8xxxu_rate_adaptive {
> u8 rssi_level; /* INIT, HIGH, MIDDLE, LOW */
> } __packed;
>
> +struct rtl8xxxu_watchdog {
> + struct ieee80211_vif *vif;
> + struct delayed_work ra_wq;
> +};
Having to store the vif address under the device-specific private
structure may be a layering violation, but I'm not fully grasping how
all this fits together. Can anyone from linux-wireless help?
The existing rtl8xxxu_add_interface() code appears to allow multiple
STA interfaces to be added. Does that imply that the hardware should
support connecting to multiple APs on different channels? I'm pretty
sure the hardware doesn't support that; if so we could do something
similar to ar5523.c where it only allows a single vif, and can easily
store that pointer in the device-specific structure.
Or if there's a valid reason to support multiple vifs, then we need to
figure out how to implement this watchdog. As shown below, the
watchdog needs to know the supported rate info of the AP you are
connected to, and the RSSI, and that comes from a specific vif. If
multiple vifs are present, how would we know which one to choose for
this rate adjustment?
> struct rtl8xxxu_priv {
> struct ieee80211_hw *hw;
> struct usb_device *udev;
> @@ -1344,6 +1349,7 @@ struct rtl8xxxu_priv {
> u8 no_pape:1;
> u8 int_buf[USB_INTR_CONTENT_LENGTH];
> struct rtl8xxxu_rate_adaptive ra_info;
> + struct rtl8xxxu_watchdog watchdog;
> };
>
> struct rtl8xxxu_rx_urb {
> @@ -1380,6 +1386,8 @@ struct rtl8xxxu_fileops {
> bool ht40);
> void (*update_rate_mask) (struct rtl8xxxu_priv *priv,
> u32 ramask, int sgi);
> + void (*refresh_rate_mask) (struct rtl8xxxu_priv *priv, int signal,
> + struct ieee80211_sta *sta);
> void (*report_connect) (struct rtl8xxxu_priv *priv,
> u8 macid, bool connect);
> void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> index 26b674aca125..92c35afecae0 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> @@ -1645,6 +1645,156 @@ static void rtl8723bu_init_statistics(struct rtl8xxxu_priv *priv)
> rtl8xxxu_write32(priv, REG_OFDM0_FA_RSTC, val32);
> }
>
> +static u8 rtl8723b_signal_to_rssi(int signal)
> +{
> + if (signal < -95)
> + signal = -95;
> + return (u8)(signal + 95);
> +}
> +
> +static void rtl8723b_refresh_rate_mask(struct rtl8xxxu_priv *priv,
> + int signal, struct ieee80211_sta *sta)
> +{
> + struct rtl8xxxu_rate_adaptive *ra;
> + struct ieee80211_hw *hw = priv->hw;
> + u16 wireless_mode;
> + u8 rssi_level, ratr_index;
> + u8 txbw_40mhz;
> + u8 rssi, rssi_thresh_high, rssi_thresh_low;
> +
> + ra = &priv->ra_info;
> + wireless_mode = ra->wireless_mode;
> + rssi_level = ra->rssi_level;
> + rssi = rtl8723b_signal_to_rssi(signal);
> + ratr_index = ra->ratr_index;
> + txbw_40mhz = (hw->conf.chandef.width == NL80211_CHAN_WIDTH_40)? 1 : 0;
> +
> + switch (rssi_level) {
> + case RTL8XXXU_RATR_STA_HIGH:
> + rssi_thresh_high = 50;
> + rssi_thresh_low = 20;
> + break;
> + case RTL8XXXU_RATR_STA_MID:
> + rssi_thresh_high = 55;
> + rssi_thresh_low = 20;
> + break;
> + case RTL8XXXU_RATR_STA_LOW:
> + rssi_thresh_high = 60;
> + rssi_thresh_low = 25;
> + break;
> + default:
> + rssi_thresh_high = 50;
> + rssi_thresh_low = 20;
> + break;
> + }
> +
> + if (rssi > rssi_thresh_high)
> + rssi_level = RTL8XXXU_RATR_STA_HIGH;
> + else if (rssi > rssi_thresh_low)
> + rssi_level = RTL8XXXU_RATR_STA_MID;
> + else
> + rssi_level = RTL8XXXU_RATR_STA_LOW;
> +
> + if (rssi_level != ra->rssi_level) {
> + int sgi = 0;
> + u32 rate_bitmap = 0;
> +
> + rcu_read_lock();
> + rate_bitmap = (sta->supp_rates[0] & 0xfff) |
> + sta->ht_cap.mcs.rx_mask[0] << 12 |
> + sta->ht_cap.mcs.rx_mask[1] << 20;
> + if (sta->ht_cap.cap &
> + (IEEE80211_HT_CAP_SGI_40 | IEEE80211_HT_CAP_SGI_20))
> + sgi = 1;
> + rcu_read_unlock();
> +
> + switch (wireless_mode) {
> + case WIRELESS_MODE_B:
> + ratr_index = RATEID_IDX_B;
> + if (rate_bitmap & 0x0000000c)
> + rate_bitmap &= 0x0000000d;
> + else
> + rate_bitmap &= 0x0000000f;
> + break;
> + case WIRELESS_MODE_A:
> + case WIRELESS_MODE_G:
> + ratr_index = RATEID_IDX_G;
> + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> + rate_bitmap &= 0x00000f00;
> + else
> + rate_bitmap &= 0x00000ff0;
> + break;
> + case (WIRELESS_MODE_B|WIRELESS_MODE_G):
> + ratr_index = RATEID_IDX_BG;
> + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> + rate_bitmap &= 0x00000f00;
> + else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> + rate_bitmap &= 0x00000ff0;
> + else
> + rate_bitmap &= 0x00000ff5;
> + break;
> + case WIRELESS_MODE_N_24G:
> + case WIRELESS_MODE_N_5G:
> + case (WIRELESS_MODE_G|WIRELESS_MODE_N_24G):
> + case (WIRELESS_MODE_A|WIRELESS_MODE_N_5G):
> + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> + ratr_index = RATEID_IDX_GN_N2SS;
> + else
> + ratr_index = RATEID_IDX_GN_N1SS;
> + case (WIRELESS_MODE_B|WIRELESS_MODE_G|WIRELESS_MODE_N_24G):
> + case (WIRELESS_MODE_B|WIRELESS_MODE_N_24G):
> + if (txbw_40mhz) {
> + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> + ratr_index = RATEID_IDX_BGN_40M_2SS;
> + else
> + ratr_index = RATEID_IDX_BGN_40M_1SS;
> + }
> + else {
> + if (priv->tx_paths == 2 && priv->rx_paths == 2)
> + ratr_index = RATEID_IDX_BGN_20M_2SS_BN;
> + else
> + ratr_index = RATEID_IDX_BGN_20M_1SS_BN;
> + }
> +
> + if (priv->tx_paths == 2 && priv->rx_paths == 2) {
> + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> + rate_bitmap &= 0x0f8f0000;
> + else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> + rate_bitmap &= 0x0f8ff000;
> + else {
> + if (txbw_40mhz)
> + rate_bitmap &= 0x0f8ff015;
> + else
> + rate_bitmap &= 0x0f8ff005;
> + }
> + }
> + else {
> + if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> + rate_bitmap &= 0x000f0000;
> + else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> + rate_bitmap &= 0x000ff000;
> + else {
> + if (txbw_40mhz)
> + rate_bitmap &= 0x000ff015;
> + else
> + rate_bitmap &= 0x000ff005;
> + }
> + }
> + break;
> + default:
> + ratr_index = RATEID_IDX_BGN_40M_2SS;
> + rate_bitmap &= 0x0fffffff;
> + break;
> + }
> +
> + ra->ratr_index = ratr_index;
> + ra->rssi_level = rssi_level;
> + priv->fops->update_rate_mask(priv, rate_bitmap, sgi);
> + }
> +
> + return;
> +}
> +
> struct rtl8xxxu_fileops rtl8723bu_fops = {
> .parse_efuse = rtl8723bu_parse_efuse,
> .load_firmware = rtl8723bu_load_firmware,
> @@ -1665,6 +1815,7 @@ struct rtl8xxxu_fileops rtl8723bu_fops = {
> .usb_quirks = rtl8xxxu_gen2_usb_quirks,
> .set_tx_power = rtl8723b_set_tx_power,
> .update_rate_mask = rtl8xxxu_gen2_update_rate_mask,
> + .refresh_rate_mask = rtl8723b_refresh_rate_mask,
> .report_connect = rtl8xxxu_gen2_report_connect,
> .fill_txdesc = rtl8xxxu_fill_txdesc_v2,
> .writeN_block_size = 1024,
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 360e9bd837e5..8db479986e97 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4565,6 +4565,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> sgi = 1;
> rcu_read_unlock();
>
> + priv->watchdog.vif = vif;
> ra = &priv->ra_info;
> ra->wireless_mode = rtl8xxxu_wireless_mode(hw, sta);
> ra->ratr_index = RATEID_IDX_BGN_40M_2SS;
> @@ -5822,6 +5823,38 @@ rtl8xxxu_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> return 0;
> }
>
> +static void rtl8xxxu_watchdog_callback(struct work_struct *work)
> +{
> + struct ieee80211_vif *vif;
> + struct rtl8xxxu_watchdog *wdog;
> + struct rtl8xxxu_priv *priv;
> +
> + wdog = container_of(work, struct rtl8xxxu_watchdog, ra_wq.work);
> + priv = container_of(wdog, struct rtl8xxxu_priv, watchdog);
> + vif = wdog->vif;
> +
> + if (vif) {
> + int signal;
> + struct ieee80211_sta *sta;
> +
> + rcu_read_lock();
Can you explain the lock/unlock here?
> + sta = ieee80211_find_sta(vif, vif->bss_conf.bssid);
> + if (!sta) {
> + struct device *dev = &priv->udev->dev;
> + dev_info(dev, "%s: no sta found\n", __func__);
Does this result in a kernel log message every 2 seconds when the wifi
interface is not associated to an AP?
> + rcu_read_unlock();
> + return;
> + }
> + rcu_read_unlock();
> +
> + signal = ieee80211_ave_rssi(vif);
> + if (priv->fops->refresh_rate_mask)
> + priv->fops->refresh_rate_mask(priv, signal, sta);
> + }
> +
> + schedule_delayed_work(&priv->watchdog.ra_wq, 2 * HZ);
> +}
> +
> static int rtl8xxxu_start(struct ieee80211_hw *hw)
> {
> struct rtl8xxxu_priv *priv = hw->priv;
> @@ -5878,6 +5911,8 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
>
> ret = rtl8xxxu_submit_rx_urb(priv, rx_urb);
> }
> +
> + schedule_delayed_work(&priv->watchdog.ra_wq, 2* HZ);
> exit:
> /*
> * Accept all data and mgmt frames
> @@ -6101,6 +6136,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> INIT_LIST_HEAD(&priv->rx_urb_pending_list);
> spin_lock_init(&priv->rx_urb_lock);
> INIT_WORK(&priv->rx_urb_wq, rtl8xxxu_rx_urb_work);
> + INIT_DELAYED_WORK(&priv->watchdog.ra_wq, rtl8xxxu_watchdog_callback);
>
> usb_set_intfdata(interface, hw);
>
> @@ -6226,6 +6262,8 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
> mutex_destroy(&priv->usb_buf_mutex);
> mutex_destroy(&priv->h2c_mutex);
>
> + cancel_delayed_work_sync(&priv->watchdog.ra_wq);
> +
> if (priv->udev->state != USB_STATE_NOTATTACHED) {
> dev_info(&priv->udev->dev,
> "Device still attached, trying to reset\n");
> --
> 2.21.0
>
^ permalink raw reply
* [PATCH v2 12/17] mt7615: mcu: do not use function pointers whenever possible
From: Lorenzo Bianconi @ 2019-05-09 7:54 UTC (permalink / raw)
To: nbd; +Cc: lorenzo.bianconi, linux-wireless, ryder.lee, royluo
In-Reply-To: <cover.1557388046.git.lorenzo@kernel.org>
Remove function pointers in mt7615_mcu_set_bss_info and run function
directly. Moreover remove __mt7615_mcu_set_bss_info since it is run just
by mt7615_mcu_set_bss_info and remove duplicated istructions
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
Changes since v1:
- fix tag computation in mt7615_mcu_set_bss_info
---
.../net/wireless/mediatek/mt76/mt7615/mcu.c | 220 +++++++++---------
1 file changed, 105 insertions(+), 115 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index 0e82fcb34e07..2ef4e4ef3a78 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -676,154 +676,107 @@ int mt7615_mcu_set_dev_info(struct mt7615_dev *dev,
return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_DEV_INFO_UPDATE);
}
-static void bss_info_omac_handler (struct mt7615_dev *dev,
- struct bss_info *bss_info,
- struct sk_buff *skb)
+static void
+mt7615_mcu_bss_info_omac_header(struct mt7615_vif *mvif, u8 *data,
+ u32 conn_type)
{
- struct bss_info_omac tlv = {0};
-
- tlv.tag = cpu_to_le16(BSS_INFO_OMAC);
- tlv.len = cpu_to_le16(sizeof(tlv));
- tlv.hw_bss_idx = (bss_info->omac_idx > EXT_BSSID_START) ?
- HW_BSSID_0 : bss_info->omac_idx;
- tlv.omac_idx = bss_info->omac_idx;
- tlv.band_idx = bss_info->band_idx;
- tlv.conn_type = cpu_to_le32(bss_info->conn_type);
-
- memcpy(skb_put(skb, sizeof(tlv)), &tlv, sizeof(tlv));
+ struct bss_info_omac *hdr = (struct bss_info_omac *)data;
+ u8 idx;
+
+ idx = mvif->omac_idx > EXT_BSSID_START ? HW_BSSID_0 : mvif->omac_idx;
+ hdr->tag = cpu_to_le16(BSS_INFO_OMAC);
+ hdr->len = cpu_to_le16(sizeof(struct bss_info_omac));
+ hdr->hw_bss_idx = idx;
+ hdr->omac_idx = mvif->omac_idx;
+ hdr->band_idx = mvif->band_idx;
+ hdr->conn_type = cpu_to_le32(conn_type);
}
-static void bss_info_basic_handler (struct mt7615_dev *dev,
- struct bss_info *bss_info,
- struct sk_buff *skb)
+static void
+mt7615_mcu_bss_info_basic_header(struct ieee80211_vif *vif, u8 *data,
+ u32 net_type, u8 tx_wlan_idx,
+ bool enable)
{
- struct bss_info_basic tlv = {0};
-
- tlv.tag = cpu_to_le16(BSS_INFO_BASIC);
- tlv.len = cpu_to_le16(sizeof(tlv));
- tlv.network_type = cpu_to_le32(bss_info->network_type);
- tlv.active = bss_info->enable;
- tlv.bcn_interval = cpu_to_le16(bss_info->bcn_interval);
- memcpy(tlv.bssid, bss_info->bssid, ETH_ALEN);
- tlv.wmm_idx = bss_info->wmm_idx;
- tlv.dtim_period = bss_info->dtim_period;
- tlv.bmc_tx_wlan_idx = bss_info->bmc_tx_wlan_idx;
-
- memcpy(skb_put(skb, sizeof(tlv)), &tlv, sizeof(tlv));
+ struct mt7615_vif *mvif = (struct mt7615_vif *)vif->drv_priv;
+ struct bss_info_basic *hdr = (struct bss_info_basic *)data;
+
+ hdr->tag = cpu_to_le16(BSS_INFO_BASIC);
+ hdr->len = cpu_to_le16(sizeof(struct bss_info_basic));
+ hdr->network_type = cpu_to_le32(net_type);
+ hdr->active = enable;
+ hdr->bcn_interval = cpu_to_le16(vif->bss_conf.beacon_int);
+ memcpy(hdr->bssid, vif->bss_conf.bssid, ETH_ALEN);
+ hdr->wmm_idx = mvif->wmm_idx;
+ hdr->dtim_period = vif->bss_conf.dtim_period;
+ hdr->bmc_tx_wlan_idx = tx_wlan_idx;
}
-static void bss_info_ext_bss_handler (struct mt7615_dev *dev,
- struct bss_info *bss_info,
- struct sk_buff *skb)
+static void
+mt7615_mcu_bss_info_ext_header(struct mt7615_vif *mvif, u8 *data)
{
/* SIFS 20us + 512 byte beacon tranmitted by 1Mbps (3906us) */
#define BCN_TX_ESTIMATE_TIME (4096 + 20)
- struct bss_info_ext_bss tlv = {0};
+ struct bss_info_ext_bss *hdr = (struct bss_info_ext_bss *)data;
int ext_bss_idx;
- ext_bss_idx = bss_info->omac_idx - EXT_BSSID_START;
-
+ ext_bss_idx = mvif->omac_idx - EXT_BSSID_START;
if (ext_bss_idx < 0)
return;
- tlv.tag = cpu_to_le16(BSS_INFO_EXT_BSS);
- tlv.len = cpu_to_le16(sizeof(tlv));
- tlv.mbss_tsf_offset = ext_bss_idx * BCN_TX_ESTIMATE_TIME;
-
- memcpy(skb_put(skb, sizeof(tlv)), &tlv, sizeof(tlv));
+ hdr->tag = cpu_to_le16(BSS_INFO_EXT_BSS);
+ hdr->len = cpu_to_le16(sizeof(struct bss_info_ext_bss));
+ hdr->mbss_tsf_offset = ext_bss_idx * BCN_TX_ESTIMATE_TIME;
}
-static struct bss_info_tag_handler bss_info_tag_handler[] = {
- {BSS_INFO_OMAC, sizeof(struct bss_info_omac), bss_info_omac_handler},
- {BSS_INFO_BASIC, sizeof(struct bss_info_basic), bss_info_basic_handler},
- {BSS_INFO_RF_CH, sizeof(struct bss_info_rf_ch), NULL},
- {BSS_INFO_PM, 0, NULL},
- {BSS_INFO_UAPSD, 0, NULL},
- {BSS_INFO_ROAM_DETECTION, 0, NULL},
- {BSS_INFO_LQ_RM, 0, NULL},
- {BSS_INFO_EXT_BSS, sizeof(struct bss_info_ext_bss), bss_info_ext_bss_handler},
- {BSS_INFO_BMC_INFO, 0, NULL},
- {BSS_INFO_SYNC_MODE, 0, NULL},
- {BSS_INFO_RA, 0, NULL},
- {BSS_INFO_MAX_NUM, 0, NULL},
-};
-
-static int __mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
- struct bss_info *bss_info)
+int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
+ struct ieee80211_vif *vif, int en)
{
+ struct mt7615_vif *mvif = (struct mt7615_vif *)vif->drv_priv;
struct req_hdr {
u8 bss_idx;
u8 rsv0;
__le16 tlv_num;
u8 is_tlv_append;
u8 rsv1[3];
- } __packed req_hdr = {0};
+ } __packed;
+ int len = sizeof(struct req_hdr) + sizeof(struct bss_info_basic);
+ int ret, i, features = BIT(BSS_INFO_BASIC), ntlv = 1;
+ u32 conn_type = 0, net_type = NETWORK_INFRA;
+ u8 *buf, *data, tx_wlan_idx = 0;
+ struct req_hdr *hdr;
struct sk_buff *skb;
- u16 tlv_num = 0;
- u32 size = 0;
- int i;
- for (i = 0; i < BSS_INFO_MAX_NUM; i++)
- if ((BIT(bss_info_tag_handler[i].tag) & bss_info->feature) &&
- bss_info_tag_handler[i].handler) {
- tlv_num++;
- size += bss_info_tag_handler[i].len;
+ if (en) {
+ len += sizeof(struct bss_info_omac);
+ features |= BIT(BSS_INFO_OMAC);
+ if (mvif->omac_idx > EXT_BSSID_START) {
+ len += sizeof(struct bss_info_ext_bss);
+ features |= BIT(BSS_INFO_EXT_BSS);
+ ntlv++;
}
-
- skb = mt7615_mcu_msg_alloc(NULL, sizeof(req_hdr) + size);
-
- req_hdr.bss_idx = bss_info->bss_idx;
- req_hdr.tlv_num = cpu_to_le16(tlv_num);
- req_hdr.is_tlv_append = tlv_num ? 1 : 0;
-
- memcpy(skb_put(skb, sizeof(req_hdr)), &req_hdr, sizeof(req_hdr));
-
- for (i = 0; i < BSS_INFO_MAX_NUM; i++)
- if ((BIT(bss_info_tag_handler[i].tag) & bss_info->feature) &&
- bss_info_tag_handler[i].handler)
- bss_info_tag_handler[i].handler(dev, bss_info, skb);
-
- return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BSS_INFO_UPDATE);
-}
-
-int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
- struct ieee80211_vif *vif, int en)
-{
- struct mt7615_vif *mvif = (struct mt7615_vif *)vif->drv_priv;
- struct bss_info bss_info = {
- .bss_idx = mvif->idx,
- .omac_idx = mvif->omac_idx,
- .band_idx = mvif->band_idx,
- .bcn_interval = vif->bss_conf.beacon_int,
- .dtim_period = vif->bss_conf.dtim_period,
- .enable = en,
- .feature = BIT(BSS_INFO_BASIC),
- .wmm_idx = mvif->wmm_idx,
- };
+ ntlv++;
+ }
switch (vif->type) {
case NL80211_IFTYPE_AP:
- bss_info.bmc_tx_wlan_idx = mvif->sta.wcid.idx;
- bss_info.network_type = NETWORK_INFRA;
- bss_info.conn_type = CONNECTION_INFRA_AP;
+ tx_wlan_idx = mvif->sta.wcid.idx;
+ conn_type = CONNECTION_INFRA_AP;
break;
case NL80211_IFTYPE_STATION: {
- /* find the unicast entry for sta mode bmc tx */
- struct ieee80211_sta *ap_sta;
+ struct ieee80211_sta *sta;
struct mt7615_sta *msta;
rcu_read_lock();
- ap_sta = ieee80211_find_sta(vif, vif->bss_conf.bssid);
- if (!ap_sta) {
+ sta = ieee80211_find_sta(vif, vif->bss_conf.bssid);
+ if (!sta) {
rcu_read_unlock();
return -EINVAL;
}
- msta = (struct mt7615_sta *)ap_sta->drv_priv;
- bss_info.bmc_tx_wlan_idx = msta->wcid.idx;
- bss_info.network_type = NETWORK_INFRA;
- bss_info.conn_type = CONNECTION_INFRA_STA;
+ msta = (struct mt7615_sta *)sta->drv_priv;
+ tx_wlan_idx = msta->wcid.idx;
+ conn_type = CONNECTION_INFRA_STA;
rcu_read_unlock();
break;
@@ -832,15 +785,52 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
WARN_ON(1);
break;
}
- memcpy(bss_info.bssid, vif->bss_conf.bssid, ETH_ALEN);
- if (en) {
- bss_info.feature |= BIT(BSS_INFO_OMAC);
- if (mvif->omac_idx > EXT_BSSID_START)
- bss_info.feature |= BIT(BSS_INFO_EXT_BSS);
+ buf = kzalloc(len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ hdr = (struct req_hdr *)buf;
+ hdr->bss_idx = mvif->idx;
+ hdr->tlv_num = cpu_to_le16(ntlv);
+ hdr->is_tlv_append = 1;
+
+ data = buf + sizeof(*hdr);
+ for (i = 0; i < BSS_INFO_MAX_NUM; i++) {
+ int tag = ffs(features & BIT(i)) - 1;
+
+ switch (tag) {
+ case BSS_INFO_OMAC:
+ mt7615_mcu_bss_info_omac_header(mvif, data,
+ conn_type);
+ data += sizeof(struct bss_info_omac);
+ break;
+ case BSS_INFO_BASIC:
+ mt7615_mcu_bss_info_basic_header(vif, data, net_type,
+ tx_wlan_idx, en);
+ data += sizeof(struct bss_info_basic);
+ break;
+ case BSS_INFO_EXT_BSS:
+ mt7615_mcu_bss_info_ext_header(mvif, data);
+ data += sizeof(struct bss_info_ext_bss);
+ break;
+ default:
+ break;
+ }
+ }
+
+ skb = mt7615_mcu_msg_alloc(buf, len);
+ if (!skb) {
+ ret = -ENOMEM;
+ goto out;
}
- return __mt7615_mcu_set_bss_info(dev, &bss_info);
+ ret = mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BSS_INFO_UPDATE);
+
+out:
+ kfree(buf);
+
+ return ret;
}
static int
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes
From: Randy Dunlap @ 2019-05-09 0:58 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: linux-fbdev, Stanislaw Gruszka, Petr Mladek, David Airlie,
Joonas Lahtinen, dri-devel, devel, linux-scsi, Jassi Brar, ath10k,
intel-gfx, Dan Carpenter, Jose Abreu, Tom Lendacky,
James E.J. Bottomley, Jani Nikula, linux-fsdevel, Steven Rostedt,
Rodrigo Vivi, Benson Leung, Kalle Valo, Karsten Keil,
Martin K. Petersen, Greg Kroah-Hartman, linux-wireless,
linux-kernel, Sergey Senozhatsky, David Laight, Daniel Vetter,
netdev, Enric Balletbo i Serra, Andrew Morton, David S. Miller,
Alexander Viro
In-Reply-To: <20190508070148.23130-4-alastair@au1.ibm.com>
On 5/8/19 12:01 AM, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Some buffers may only be partially filled with useful data, while the rest
> is padded (typically with 0x00 or 0xff).
>
> This patch introduces a flag to allow the supression of lines of repeated
> bytes, which are replaced with '** Skipped %u bytes of value 0x%x **'
>
> An inline wrapper function is provided for backwards compatibility with
> existing code, which maintains the original behaviour.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> include/linux/printk.h | 25 +++++++++---
> lib/hexdump.c | 91 ++++++++++++++++++++++++++++++++++++------
> 2 files changed, 99 insertions(+), 17 deletions(-)
>
Hi,
Did you do "make htmldocs" or something similar on this?
> diff --git a/lib/hexdump.c b/lib/hexdump.c
> index 3943507bc0e9..d61a1e4f19fa 100644
> --- a/lib/hexdump.c
> +++ b/lib/hexdump.c
> @@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> EXPORT_SYMBOL(hex_dump_to_buffer);
>
> #ifdef CONFIG_PRINTK
> +
> +/**
> + * Check if a buffer contains only a single byte value
> + * @buf: pointer to the buffer
> + * @len: the size of the buffer in bytes
> + * @val: outputs the value if if the bytes are identical
Does this work without a function name?
Documentation/doc-guide/kernel-doc.rst says the general format is:
/**
* function_name() - Brief description of function.
* @arg1: Describe the first argument.
* @arg2: Describe the second argument.
* One can provide multiple line descriptions
* for arguments.
*
> + */
> /**
> - * print_hex_dump - print a text hex dump to syslog for a binary blob of data
> + * print_hex_dump_ext: dump a binary blob of data to syslog in hexadecimal
Also not in the general documented format.
> * @level: kernel log level (e.g. KERN_DEBUG)
> * @prefix_str: string to prefix each line with;
> * caller supplies trailing spaces for alignment if desired
--
~Randy
^ permalink raw reply
* Re: [PATCH 03/16] lib,treewide: add new match_string() helper/macro
From: Ardelean, Alexandru @ 2019-05-08 13:22 UTC (permalink / raw)
To: andriy.shevchenko@linux.intel.com, gregkh@linuxfoundation.org
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-ide@vger.kernel.org, linux-usb@vger.kernel.org,
linux-mmc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
cgroups@vger.kernel.org, intel-gfx@lists.freedesktop.org,
linux-pm@vger.kernel.org, linux-mm@kvack.org,
linux-omap@vger.kernel.org, linux-gpio@vger.kernel.org,
linux-security-module@vger.kernel.org, devel@driverdev.osuosl.org,
linux-integrity@vger.kernel.org, linux-fbdev@vger.kernel.org,
dri-devel@lists.freedesktop.org,
linux-rpi-kernel@lists.infradead.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
alsa-devel@alsa-project.org, linux-rockchip@lists.infradead.org,
linux-clk@vger.kernel.org, linux-pci@vger.kernel.org,
linux-wireless@vger.kernel.org, linux-mtd@lists.infradead.org,
linux-tegra@vger.kernel.org
In-Reply-To: <20190508131856.GB10138@kroah.com>
On Wed, 2019-05-08 at 15:18 +0200, Greg KH wrote:
>
>
> On Wed, May 08, 2019 at 04:11:28PM +0300, Andy Shevchenko wrote:
> > On Wed, May 08, 2019 at 02:28:29PM +0300, Alexandru Ardelean wrote:
> > > This change re-introduces `match_string()` as a macro that uses
> > > ARRAY_SIZE() to compute the size of the array.
> > > The macro is added in all the places that do
> > > `match_string(_a, ARRAY_SIZE(_a), s)`, since the change is pretty
> > > straightforward.
> >
> > Can you split include/linux/ change from the rest?
>
> That would break the build, why do you want it split out? This makes
> sense all as a single patch to me.
>
Not really.
It would be just be the new match_string() helper/macro in a new commit.
And the conversions of the simple users of match_string() (the ones using
ARRAY_SIZE()) in another commit.
Thanks
Alex
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH 03/16] lib,treewide: add new match_string() helper/macro
From: Greg KH @ 2019-05-08 13:18 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Alexandru Ardelean, linuxppc-dev, linux-kernel, linux-ide,
linux-clk, linux-rpi-kernel, linux-arm-kernel, linux-rockchip,
linux-pm, linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel
In-Reply-To: <20190508131128.GL9224@smile.fi.intel.com>
On Wed, May 08, 2019 at 04:11:28PM +0300, Andy Shevchenko wrote:
> On Wed, May 08, 2019 at 02:28:29PM +0300, Alexandru Ardelean wrote:
> > This change re-introduces `match_string()` as a macro that uses
> > ARRAY_SIZE() to compute the size of the array.
> > The macro is added in all the places that do
> > `match_string(_a, ARRAY_SIZE(_a), s)`, since the change is pretty
> > straightforward.
>
> Can you split include/linux/ change from the rest?
That would break the build, why do you want it split out? This makes
sense all as a single patch to me.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 03/16] lib,treewide: add new match_string() helper/macro
From: Andy Shevchenko @ 2019-05-08 13:11 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel, gregkh
In-Reply-To: <20190508112842.11654-5-alexandru.ardelean@analog.com>
On Wed, May 08, 2019 at 02:28:29PM +0300, Alexandru Ardelean wrote:
> This change re-introduces `match_string()` as a macro that uses
> ARRAY_SIZE() to compute the size of the array.
> The macro is added in all the places that do
> `match_string(_a, ARRAY_SIZE(_a), s)`, since the change is pretty
> straightforward.
Can you split include/linux/ change from the rest?
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> drivers/clk/bcm/clk-bcm2835.c | 4 +---
> drivers/gpio/gpiolib-of.c | 2 +-
> drivers/gpu/drm/i915/intel_pipe_crc.c | 2 +-
> drivers/mfd/omap-usb-host.c | 2 +-
> drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 2 +-
> drivers/pci/pcie/aer.c | 2 +-
> drivers/usb/common/common.c | 4 ++--
> drivers/usb/typec/class.c | 8 +++-----
> drivers/usb/typec/tps6598x.c | 2 +-
> drivers/vfio/vfio.c | 4 +---
> include/linux/string.h | 9 +++++++++
> sound/firewire/oxfw/oxfw.c | 2 +-
> sound/soc/codecs/max98088.c | 2 +-
> sound/soc/codecs/max98095.c | 2 +-
> 14 files changed, 25 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index a775f6a1f717..1ab388590ead 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -1390,9 +1390,7 @@ static struct clk_hw *bcm2835_register_clock(struct bcm2835_cprman *cprman,
> for (i = 0; i < data->num_mux_parents; i++) {
> parents[i] = data->parents[i];
>
> - ret = __match_string(cprman_parent_names,
> - ARRAY_SIZE(cprman_parent_names),
> - parents[i]);
> + ret = match_string(cprman_parent_names, parents[i]);
> if (ret >= 0)
> parents[i] = cprman->real_parent_names[ret];
> }
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 27d6f04ab58e..71e886869d78 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -279,7 +279,7 @@ static struct gpio_desc *of_find_regulator_gpio(struct device *dev, const char *
> if (!con_id)
> return ERR_PTR(-ENOENT);
>
> - i = __match_string(whitelist, ARRAY_SIZE(whitelist), con_id);
> + i = match_string(whitelist, con_id);
> if (i < 0)
> return ERR_PTR(-ENOENT);
>
> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
> index 286fad1f0e08..6fc4f3d3d1f6 100644
> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> @@ -449,7 +449,7 @@ display_crc_ctl_parse_source(const char *buf, enum intel_pipe_crc_source *s)
> return 0;
> }
>
> - i = __match_string(pipe_crc_sources, ARRAY_SIZE(pipe_crc_sources), buf);
> + i = match_string(pipe_crc_sources, buf);
> if (i < 0)
> return i;
>
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 9aaacb5bdb26..53dff34c0afc 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -509,7 +509,7 @@ static int usbhs_omap_get_dt_pdata(struct device *dev,
> continue;
>
> /* get 'enum usbhs_omap_port_mode' from port mode string */
> - ret = __match_string(port_modes, ARRAY_SIZE(port_modes), mode);
> + ret = match_string(port_modes, mode);
> if (ret < 0) {
> dev_warn(dev, "Invalid port%d-mode \"%s\" in device tree\n",
> i, mode);
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> index 59ce3ff35553..778b4dfd8b75 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> @@ -667,7 +667,7 @@ iwl_dbgfs_bt_force_ant_write(struct iwl_mvm *mvm, char *buf,
> };
> int ret, bt_force_ant_mode;
>
> - ret = __match_string(modes_str, ARRAY_SIZE(modes_str), buf);
> + ret = match_string(modes_str, buf);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 41a0773a1cbc..2278caba109c 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -203,7 +203,7 @@ void pcie_ecrc_get_policy(char *str)
> {
> int i;
>
> - i = __match_string(ecrc_policy_str, ARRAY_SIZE(ecrc_policy_str), str);
> + i = match_string(ecrc_policy_str, str);
> if (i < 0)
> return;
>
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index bca0c404c6ca..5a651d311d38 100644
> --- a/drivers/usb/common/common.c
> +++ b/drivers/usb/common/common.c
> @@ -68,7 +68,7 @@ enum usb_device_speed usb_get_maximum_speed(struct device *dev)
> if (ret < 0)
> return USB_SPEED_UNKNOWN;
>
> - ret = __match_string(speed_names, ARRAY_SIZE(speed_names), maximum_speed);
> + ret = match_string(speed_names, maximum_speed);
>
> return (ret < 0) ? USB_SPEED_UNKNOWN : ret;
> }
> @@ -106,7 +106,7 @@ static enum usb_dr_mode usb_get_dr_mode_from_string(const char *str)
> {
> int ret;
>
> - ret = __match_string(usb_dr_modes, ARRAY_SIZE(usb_dr_modes), str);
> + ret = match_string(usb_dr_modes, str);
> return (ret < 0) ? USB_DR_MODE_UNKNOWN : ret;
> }
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 4abc5a76ec51..38ac776cba8a 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1409,8 +1409,7 @@ EXPORT_SYMBOL_GPL(typec_set_pwr_opmode);
> */
> int typec_find_port_power_role(const char *name)
> {
> - return __match_string(typec_port_power_roles,
> - ARRAY_SIZE(typec_port_power_roles), name);
> + return match_string(typec_port_power_roles, name);
> }
> EXPORT_SYMBOL_GPL(typec_find_port_power_role);
>
> @@ -1424,7 +1423,7 @@ EXPORT_SYMBOL_GPL(typec_find_port_power_role);
> */
> int typec_find_power_role(const char *name)
> {
> - return __match_string(typec_roles, ARRAY_SIZE(typec_roles), name);
> + return match_string(typec_roles, name);
> }
> EXPORT_SYMBOL_GPL(typec_find_power_role);
>
> @@ -1438,8 +1437,7 @@ EXPORT_SYMBOL_GPL(typec_find_power_role);
> */
> int typec_find_port_data_role(const char *name)
> {
> - return __match_string(typec_port_data_roles,
> - ARRAY_SIZE(typec_port_data_roles), name);
> + return match_string(typec_port_data_roles, name);
> }
> EXPORT_SYMBOL_GPL(typec_find_port_data_role);
>
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index 0389e4391faf..0c4e47868590 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -423,7 +423,7 @@ static int tps6598x_check_mode(struct tps6598x *tps)
> if (ret)
> return ret;
>
> - switch (__match_string(modes, ARRAY_SIZE(modes), mode)) {
> + switch (match_string(modes, mode)) {
> case TPS_MODE_APP:
> return 0;
> case TPS_MODE_BOOT:
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index b31585ecf48f..fe8283d3781b 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -637,9 +637,7 @@ static bool vfio_dev_whitelisted(struct device *dev, struct device_driver *drv)
> return true;
> }
>
> - return __match_string(vfio_driver_whitelist,
> - ARRAY_SIZE(vfio_driver_whitelist),
> - drv->name) >= 0;
> + return match_string(vfio_driver_whitelist, drv->name) >= 0;
> }
>
> /*
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 531d04308ff9..07e9f89088df 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -194,6 +194,15 @@ static inline int strtobool(const char *s, bool *res)
> int __match_string(const char * const *array, size_t n, const char *string);
> int __sysfs_match_string(const char * const *array, size_t n, const char *s);
>
> +/**
> + * match_string - matches given string in an array
> + * @_a: array of strings
> + * @_s: string to match with
> + *
> + * Helper for __match_string(). Calculates the size of @a automatically.
> + */
> +#define match_string(_a, _s) __match_string(_a, ARRAY_SIZE(_a), _s)
> +
> /**
> * sysfs_match_string - matches given string in an array
> * @_a: array of strings
> diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
> index 9ec5316f3bb5..433fc84c4f90 100644
> --- a/sound/firewire/oxfw/oxfw.c
> +++ b/sound/firewire/oxfw/oxfw.c
> @@ -57,7 +57,7 @@ static bool detect_loud_models(struct fw_unit *unit)
> if (err < 0)
> return false;
>
> - return __match_string(models, ARRAY_SIZE(models), model) >= 0;
> + return match_string(models, model) >= 0;
> }
>
> static int name_card(struct snd_oxfw *oxfw)
> diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c
> index 3ef743075bda..911ffe84c37e 100644
> --- a/sound/soc/codecs/max98088.c
> +++ b/sound/soc/codecs/max98088.c
> @@ -1405,7 +1405,7 @@ static int max98088_get_channel(struct snd_soc_component *component, const char
> {
> int ret;
>
> - ret = __match_string(eq_mode_name, ARRAY_SIZE(eq_mode_name), name);
> + ret = match_string(eq_mode_name, name);
> if (ret < 0)
> dev_err(component->dev, "Bad EQ channel name '%s'\n", name);
> return ret;
> diff --git a/sound/soc/codecs/max98095.c b/sound/soc/codecs/max98095.c
> index cd69916d5dcb..d182d45d0c83 100644
> --- a/sound/soc/codecs/max98095.c
> +++ b/sound/soc/codecs/max98095.c
> @@ -1636,7 +1636,7 @@ static int max98095_get_bq_channel(struct snd_soc_component *component,
> {
> int ret;
>
> - ret = __match_string(bq_mode_name, ARRAY_SIZE(bq_mode_name), name);
> + ret = match_string(bq_mode_name, name);
> if (ret < 0)
> dev_err(component->dev, "Bad biquad channel name '%s'\n", name);
> return ret;
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 09/16] mmc: sdhci-xenon: use new match_string() helper/macro
From: Dan Carpenter @ 2019-05-08 12:20 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel, gregkh,
andriy.shevchenko
In-Reply-To: <20190508112842.11654-11-alexandru.ardelean@analog.com>
On Wed, May 08, 2019 at 02:28:35PM +0300, Alexandru Ardelean wrote:
> -static const char * const phy_types[] = {
> - "emmc 5.0 phy",
> - "emmc 5.1 phy"
> -};
> -
> enum xenon_phy_type_enum {
> EMMC_5_0_PHY,
> EMMC_5_1_PHY,
> NR_PHY_TYPES
There is no need for NR_PHY_TYPES now so you could remove that as well.
regards,
dan carpenter
^ permalink raw reply
* RE: [PATCH v2 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Alastair D'Silva @ 2019-05-08 11:41 UTC (permalink / raw)
To: 'David Laight', 'Alastair D'Silva'
Cc: 'Jani Nikula', 'Joonas Lahtinen',
'Rodrigo Vivi', 'David Airlie',
'Daniel Vetter', 'Dan Carpenter',
'Karsten Keil', 'Jassi Brar',
'Tom Lendacky', 'David S. Miller',
'Jose Abreu', 'Kalle Valo',
'Stanislaw Gruszka', 'Benson Leung',
'Enric Balletbo i Serra', 'James E.J. Bottomley',
'Martin K. Petersen', 'Greg Kroah-Hartman',
'Alexander Viro', 'Petr Mladek',
'Sergey Senozhatsky', 'Steven Rostedt',
'Andrew Morton', intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <c98a499a4e824bcd824d5ad53d037c67@AcuMS.aculab.com>
> -----Original Message-----
> From: David Laight <David.Laight@ACULAB.COM>
> Sent: Wednesday, 8 May 2019 7:20 PM
> To: 'Alastair D'Silva' <alastair@au1.ibm.com>; alastair@d-silva.org
> Cc: Jani Nikula <jani.nikula@linux.intel.com>; Joonas Lahtinen
> <joonas.lahtinen@linux.intel.com>; Rodrigo Vivi <rodrigo.vivi@intel.com>;
> David Airlie <airlied@linux.ie>; Daniel Vetter <daniel@ffwll.ch>; Dan
> Carpenter <dan.carpenter@oracle.com>; Karsten Keil <isdn@linux-
> pingi.de>; Jassi Brar <jassisinghbrar@gmail.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; David S. Miller <davem@davemloft.net>;
> Jose Abreu <Jose.Abreu@synopsys.com>; Kalle Valo
> <kvalo@codeaurora.org>; Stanislaw Gruszka <sgruszka@redhat.com>;
> Benson Leung <bleung@chromium.org>; Enric Balletbo i Serra
> <enric.balletbo@collabora.com>; James E.J. Bottomley
> <jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>;
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Alexander Viro
> <viro@zeniv.linux.org.uk>; Petr Mladek <pmladek@suse.com>; Sergey
> Senozhatsky <sergey.senozhatsky@gmail.com>; Steven Rostedt
> <rostedt@goodmis.org>; Andrew Morton <akpm@linux-foundation.org>;
> intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org;
> ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux-
> scsi@vger.kernel.org; linux-fbdev@vger.kernel.org;
> devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org
> Subject: RE: [PATCH v2 4/7] lib/hexdump.c: Replace ascii bool in
> hex_dump_to_buffer with flags
>
> From: Alastair D'Silva
> > Sent: 08 May 2019 08:02
> > To: alastair@d-silva.org
> ...
> > --- a/include/linux/printk.h
> > +++ b/include/linux/printk.h
> > @@ -480,13 +480,13 @@ enum {
> > DUMP_PREFIX_OFFSET
> > };
> >
> > -extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> > - int groupsize, char *linebuf, size_t linebuflen,
> > - bool ascii);
> > -
> > #define HEXDUMP_ASCII (1 << 0)
> > #define HEXDUMP_SUPPRESS_REPEATED (1 << 1)
>
> These ought to be BIT(0) and BIT(1)
Thanks, I'll address that.
>
> > +extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> > + int groupsize, char *linebuf, size_t linebuflen,
> > + u64 flags);
>
> Why 'u64 flags' ?
> How many flags do you envisage ??
> Your HEXDUMP_ASCII (etc) flags are currently signed values and might get
> sign extended causing grief.
> 'unsigned int flags' is probably sufficient.
I was trying to avoid having to change the prototype again in the future, but it's not a big deal, if enough work goes in to require more than 32 bits, it can be updated at that point.
>
> I've not really looked at the code, it seems OTT in places though.
I'll wait for more concrete criticisms here, this it a bit too vague to take any action on.
> If someone copies it somewhere where the performance matters (I've user
> space code which is dominated by its tracing!) then you don't want all the
> function calls and conditionals even if you want some of the functionality.
Calling hexdump (even in it's unaltered form) in performance critical code is always going to suck. As you mentioned before, it's all based around printf. A performance conscious user would be better off building their code around hex_asc_hi/lo instead (see lib/vsprintf.c:hex_string).
--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva msn: alastair@d-silva.org
blog: http://alastair.d-silva.org Twitter: @EvilDeece
^ permalink raw reply
* Re: [PATCH v4 02/10] dt-bindings: doc: reflect new NVMEM of_get_mac_address behaviour
From: Andrew Lunn @ 2019-05-08 11:48 UTC (permalink / raw)
To: Petr Štetiar
Cc: Rob Herring, netdev, devicetree, David S. Miller, Mark Rutland,
Vivien Didelot, Florian Fainelli, Yisen Zhuang, Salil Mehta,
Woojung Huh, Microchip Linux Driver Support, Kunihiko Hayashi,
Masahiro Yamada, Jassi Brar, Kalle Valo, Matthias Brugger,
Heiner Kallweit, Frank Rowand, Srinivas Kandagatla, Maxime Ripard,
linux-kernel@vger.kernel.org,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-wireless, moderated list:ARM/Mediatek SoC support
In-Reply-To: <20190508084152.GM81826@meh.true.cz>
> I've just tried to reword what was already there, anyway, did I understood
> your remark properly, would this be more appropriate?
>
> The MAC address will be determined using the optional properties defined in
> ethernet.txt and only if efuse-mac is set to 0. If any of the optional MAC
> address properties are not present, then the driver will use random MAC
> address.
If _all_ of the optional ...
Andrew
^ permalink raw reply
* [PATCH 01/16] lib: fix match_string() helper when array size is positive
From: Alexandru Ardelean @ 2019-05-08 11:28 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel
Cc: gregkh, andriy.shevchenko, Alexandru Ardelean
In-Reply-To: <20190508112842.11654-1-alexandru.ardelean@analog.com>
The documentation the `_match_string()` helper mentions that `n`
(size of the given array) should be:
* @n: number of strings in the array or -1 for NULL terminated arrays
The behavior of the function is different, in the sense that it exits on
the first NULL element in the array, regardless of whether `n` is -1 or a
positive number.
This patch changes the behavior, to exit the loop when a NULL element is
found and n == -1. Essentially, this aligns the behavior with the
doc-string.
There are currently many users of `match_string()`, and so, in order to go
through them, the next patches in the series will focus on doing some
cosmetic changes, which are aimed at grouping the users of
`match_string()`.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
lib/string.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/string.c b/lib/string.c
index 3ab861c1a857..76edb7bf76cb 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -648,8 +648,11 @@ int match_string(const char * const *array, size_t n, const char *string)
for (index = 0; index < n; index++) {
item = array[index];
- if (!item)
+ if (!item) {
+ if (n != (size_t)-1)
+ continue;
break;
+ }
if (!strcmp(item, string))
return index;
}
--
2.17.1
^ permalink raw reply related
* [PATCH 04/16] powerpc/xmon: use new match_string() helper/macro
From: Alexandru Ardelean @ 2019-05-08 11:28 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel
Cc: gregkh, andriy.shevchenko, Alexandru Ardelean
In-Reply-To: <20190508112842.11654-1-alexandru.ardelean@analog.com>
The change is purely cosmetic at this point in time, but it does highlight
the change done in lib/string.c for match_string().
Particularly for this change, if a regname is removed (replaced with NULL)
in the list, the match_string() helper will continue until the end of the
array and ignore the NULL.
This would technically allow for "reserved" regs, though here it's not the
case.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
arch/powerpc/xmon/xmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index efca104ac0cb..b84a7fc1112b 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3231,7 +3231,7 @@ scanhex(unsigned long *vp)
regname[i] = c;
}
regname[i] = 0;
- i = __match_string(regnames, N_PTREGS, regname);
+ i = match_string(regnames, regname);
if (i < 0) {
printf("invalid register name '%%%s'\n", regname);
return 0;
--
2.17.1
^ permalink raw reply related
* [PATCH 05/16] ALSA: oxygen: use new match_string() helper/macro
From: Alexandru Ardelean @ 2019-05-08 11:28 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel
Cc: gregkh, andriy.shevchenko, Alexandru Ardelean
In-Reply-To: <20190508112842.11654-1-alexandru.ardelean@analog.com>
The change is purely cosmetic at this point in time, but it does highlight
the change done in lib/string.c for match_string().
Particularly for this change, a control mode can be removed/added at a
different index/enum-value, and the match_string() helper will continue
until the end of the array and ignore the NULL.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
sound/pci/oxygen/oxygen_mixer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/oxygen/oxygen_mixer.c b/sound/pci/oxygen/oxygen_mixer.c
index 13c2fb75fd71..961fd1cbc712 100644
--- a/sound/pci/oxygen/oxygen_mixer.c
+++ b/sound/pci/oxygen/oxygen_mixer.c
@@ -1086,7 +1086,7 @@ static int add_controls(struct oxygen *chip,
err = snd_ctl_add(chip->card, ctl);
if (err < 0)
return err;
- j = __match_string(known_ctl_names, CONTROL_COUNT, ctl->id.name);
+ j = match_string(known_ctl_names, ctl->id.name);
if (j >= 0) {
chip->controls[j] = ctl;
ctl->private_free = oxygen_any_ctl_free;
--
2.17.1
^ permalink raw reply related
* [PATCH 06/16] x86/mtrr: use new match_string() helper + add gaps == minor fix
From: Alexandru Ardelean @ 2019-05-08 11:28 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel
Cc: gregkh, andriy.shevchenko, Alexandru Ardelean
In-Reply-To: <20190508112842.11654-1-alexandru.ardelean@analog.com>
This change is a bit more than cosmetic.
It replaces 2 values in mtrr_strings with NULL. Previously, they were
defined as "?", which is not great because you could technically pass "?",
and you would get value 2.
It's not sure whether that was intended (likely it wasn't), but this fixes
that.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
arch/x86/kernel/cpu/mtrr/if.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index 4ec7a5f7b94c..e67820a044cc 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -20,8 +20,8 @@ static const char *const mtrr_strings[MTRR_NUM_TYPES] =
{
"uncachable", /* 0 */
"write-combining", /* 1 */
- "?", /* 2 */
- "?", /* 3 */
+ NULL, /* 2 */
+ NULL, /* 3 */
"write-through", /* 4 */
"write-protect", /* 5 */
"write-back", /* 6 */
@@ -29,7 +29,9 @@ static const char *const mtrr_strings[MTRR_NUM_TYPES] =
const char *mtrr_attrib_to_str(int x)
{
- return (x <= 6) ? mtrr_strings[x] : "?";
+ if ((x >= ARRAY_SIZE(mtrr_strings)) || (mtrr_strings[x] == NULL))
+ return "?";
+ return mtrr_strings[x];
}
#ifdef CONFIG_PROC_FS
@@ -142,7 +144,7 @@ mtrr_write(struct file *file, const char __user *buf, size_t len, loff_t * ppos)
return -EINVAL;
ptr = skip_spaces(ptr + 5);
- i = __match_string(mtrr_strings, MTRR_NUM_TYPES, ptr);
+ i = match_string(mtrr_strings, ptr);
if (i < 0)
return i;
--
2.17.1
^ permalink raw reply related
* [PATCH 07/16] device connection: use new match_string() helper/macro
From: Alexandru Ardelean @ 2019-05-08 11:28 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel
Cc: gregkh, andriy.shevchenko, Alexandru Ardelean
In-Reply-To: <20190508112842.11654-1-alexandru.ardelean@analog.com>
The `device_connection` struct is defined as:
struct device_connection {
struct fwnode_handle *fwnode;
const char *endpoint[2];
const char *id;
struct list_head list;
};
The `endpoint` member is a static array of strings (on the struct), so
using the match_string() (which does an ARRAY_SIZE((con->endpoint)) should
be fine.
The recent change to match_string() (to ignore NULL entries up to the size
of the array) shouldn't affect this.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/base/devcon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index 7bc1c619b721..4a2338665585 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -70,7 +70,7 @@ void *device_connection_find_match(struct device *dev, const char *con_id,
mutex_lock(&devcon_lock);
list_for_each_entry(con, &devcon_list, list) {
- ep = __match_string(con->endpoint, 2, devname);
+ ep = match_string(con->endpoint, devname);
if (ep < 0)
continue;
--
2.17.1
^ permalink raw reply related
* [PATCH 09/16] mmc: sdhci-xenon: use new match_string() helper/macro
From: Alexandru Ardelean @ 2019-05-08 11:28 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel
Cc: gregkh, andriy.shevchenko, Alexandru Ardelean
In-Reply-To: <20190508112842.11654-1-alexandru.ardelean@analog.com>
The change is also cosmetic, but it also does a tighter coupling between
the enums & the string values. This way, the ARRAY_SIZE(phy_types) that is
implicitly done in the match_string() macro is also a bit safer.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/mmc/host/sdhci-xenon-phy.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/sdhci-xenon-phy.c b/drivers/mmc/host/sdhci-xenon-phy.c
index 59b7a6cac995..2a9206867fe1 100644
--- a/drivers/mmc/host/sdhci-xenon-phy.c
+++ b/drivers/mmc/host/sdhci-xenon-phy.c
@@ -135,17 +135,17 @@ struct xenon_emmc_phy_regs {
u32 logic_timing_val;
};
-static const char * const phy_types[] = {
- "emmc 5.0 phy",
- "emmc 5.1 phy"
-};
-
enum xenon_phy_type_enum {
EMMC_5_0_PHY,
EMMC_5_1_PHY,
NR_PHY_TYPES
};
+static const char * const phy_types[NR_PHY_TYPES] = {
+ [EMMC_5_0_PHY] = "emmc 5.0 phy",
+ [EMMC_5_1_PHY] = "emmc 5.1 phy"
+};
+
enum soc_pad_ctrl_type {
SOC_PAD_SD,
SOC_PAD_FIXED_1_8V,
@@ -821,7 +821,7 @@ static int xenon_add_phy(struct device_node *np, struct sdhci_host *host,
struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
int ret;
- priv->phy_type = __match_string(phy_types, NR_PHY_TYPES, phy_name);
+ priv->phy_type = match_string(phy_types, phy_name);
if (priv->phy_type < 0) {
dev_err(mmc_dev(host->mmc),
"Unable to determine PHY name %s. Use default eMMC 5.1 PHY\n",
--
2.17.1
^ permalink raw reply related
* [PATCH 10/16] pinctrl: armada-37xx: use new match_string() helper/macro
From: Alexandru Ardelean @ 2019-05-08 11:28 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel
Cc: gregkh, andriy.shevchenko, Alexandru Ardelean
In-Reply-To: <20190508112842.11654-1-alexandru.ardelean@analog.com>
The change is mostly cosmetic.
The `armada_37xx_pin_group` struct is defined as.
struct armada_37xx_pin_group {
const char *name;
unsigned int start_pin;
unsigned int npins;
u32 reg_mask;
u32 val[NB_FUNCS];
unsigned int extra_pin;
unsigned int extra_npins;
const char *funcs[NB_FUNCS];
unsigned int *pins;
};
The `funcs` field is a static array of strings, so using the
new `match_string()` helper (which does an implicit ARRAY_SIZE(gp->funcs))
should be fine.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 07a5bcaa0067..68b0db5ef5e9 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -348,7 +348,7 @@ static int armada_37xx_pmx_set_by_name(struct pinctrl_dev *pctldev,
dev_dbg(info->dev, "enable function %s group %s\n",
name, grp->name);
- func = __match_string(grp->funcs, NB_FUNCS, name);
+ func = match_string(grp->funcs, name);
if (func < 0)
return -ENOTSUPP;
@@ -938,7 +938,7 @@ static int armada_37xx_fill_func(struct armada_37xx_pinctrl *info)
struct armada_37xx_pin_group *gp = &info->groups[g];
int f;
- f = __match_string(gp->funcs, NB_FUNCS, name);
+ f = match_string(gp->funcs, name);
if (f < 0)
continue;
--
2.17.1
^ permalink raw reply related
* [PATCH 11/16] mm/vmpressure.c: use new match_string() helper/macro
From: Alexandru Ardelean @ 2019-05-08 11:28 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel
Cc: gregkh, andriy.shevchenko, Alexandru Ardelean
In-Reply-To: <20190508112842.11654-1-alexandru.ardelean@analog.com>
__match_string() is called on 2 static array of strings in this file. For
this reason, the conversion to the new match_string() macro/helper, was
done in this separate commit.
Using the new match_string() helper is mostly a cosmetic change (at this
point in time). The sizes of the arrays will be computed automatically,
which would only help if they ever get expanded.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
mm/vmpressure.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/vmpressure.c b/mm/vmpressure.c
index d43f33139568..b8149924f078 100644
--- a/mm/vmpressure.c
+++ b/mm/vmpressure.c
@@ -378,7 +378,7 @@ int vmpressure_register_event(struct mem_cgroup *memcg,
/* Find required level */
token = strsep(&spec, ",");
- level = __match_string(vmpressure_str_levels, VMPRESSURE_NUM_LEVELS, token);
+ level = match_string(vmpressure_str_levels, token);
if (level < 0) {
ret = level;
goto out;
@@ -387,7 +387,7 @@ int vmpressure_register_event(struct mem_cgroup *memcg,
/* Find optional mode */
token = strsep(&spec, ",");
if (token) {
- mode = __match_string(vmpressure_str_modes, VMPRESSURE_NUM_MODES, token);
+ mode = match_string(vmpressure_str_modes, token);
if (mode < 0) {
ret = mode;
goto out;
--
2.17.1
^ permalink raw reply related
* [PATCH 13/16] drm/edid: use new match_string() helper/macro
From: Alexandru Ardelean @ 2019-05-08 11:28 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, linux-ide, linux-clk,
linux-rpi-kernel, linux-arm-kernel, linux-rockchip, linux-pm,
linux-gpio, dri-devel, intel-gfx, linux-omap, linux-mmc,
linux-wireless, netdev, linux-pci, linux-tegra, devel, linux-usb,
kvm, linux-fbdev, linux-mtd, cgroups, linux-mm,
linux-security-module, linux-integrity, alsa-devel
Cc: gregkh, andriy.shevchenko, Alexandru Ardelean
In-Reply-To: <20190508112842.11654-1-alexandru.ardelean@analog.com>
The `generic_edid_name` is a static array of strings.
Using match_string() (which computes the array size via ARRAY_SIZE())
is possible.
The change is mostly cosmetic.
No functionality change.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/gpu/drm/drm_edid_load.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 1450051972ea..66e1e325ff37 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -186,7 +186,7 @@ static void *edid_load(struct drm_connector *connector, const char *name,
int i, valid_extensions = 0;
bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & DRM_UT_KMS);
- builtin = __match_string(generic_edid_name, GENERIC_EDIDS, name);
+ builtin = match_string(generic_edid_name, name);
if (builtin >= 0) {
fwdata = generic_edid[builtin];
fwsize = sizeof(generic_edid[builtin]);
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox