Messages in this thread | | | Subject | Re: [PATCH v4 03/17] wlcore: Align reg_ch_conf_pending and tmp_ch_bitmap to unsigned long for better performance | From | Paolo Bonzini <> | Date | Mon, 4 Mar 2019 16:54:23 +0100 |
| |
On 04/03/19 15:40, Fenghua Yu wrote:
>> + u32 tmp_ch_bitmap[2] __aligned(sizeof(unsigned long)); > > Now __aligned() is unnecessary because __set_bit_le() handles tmp_ch_bitmap, > right?
It's still needed because, as explained by Peter, bitmap functions should always operate on properly aligned variables.
>> struct wiphy *wiphy = wl->hw->wiphy; >> struct ieee80211_supported_band *band; >> bool timeout = false; >> @@ -1717,7 +1717,7 @@ int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl) >> >> wl1271_debug(DEBUG_CMD, "cmd reg domain config"); >> >> - memset(tmp_ch_bitmap, 0, sizeof(tmp_ch_bitmap)); >> + memcpy(tmp_ch_bitmap, wl->reg_ch_conf_pending, sizeof(tmp_ch_bitmap)); >> >> for (b = NL80211_BAND_2GHZ; b <= NL80211_BAND_5GHZ; b++) { >> band = wiphy->bands[b]; >> @@ -1738,13 +1738,10 @@ int wlcore_cmd_regdomain_config_locked(struct wl1271 *wl) >> if (ch_bit_idx < 0) >> continue; >> >> - set_bit(ch_bit_idx, (long *)tmp_ch_bitmap); >> + __set_bit_le(ch_bit_idx, (long *)tmp_ch_bitmap); > > Is __test_and_set_bit_le() more meaningful to avoid duplicate bit setting ?
No, since the return value would be unused.
Note that this patch is missing the removal of cpu_to_le32, as noticed by Peter.
Thanks,
Paolo
> >> } >> } >> >> - tmp_ch_bitmap[0] |= wl->reg_ch_conf_pending[0]; >> - tmp_ch_bitmap[1] |= wl->reg_ch_conf_pending[1]; >> - >> if (!memcmp(tmp_ch_bitmap, wl->reg_ch_conf_last, sizeof(tmp_ch_bitmap))) >> goto out; >> >> diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h >> index dd14850b0603..870eea3e7a27 100644 >> --- a/drivers/net/wireless/ti/wlcore/wlcore.h >> +++ b/drivers/net/wireless/ti/wlcore/wlcore.h >> @@ -320,9 +320,9 @@ struct wl1271 { >> bool watchdog_recovery; >> >> /* Reg domain last configuration */ >> - u32 reg_ch_conf_last[2] __aligned(8); >> + DECLARE_BITMAP(reg_ch_conf_last, 64); >> /* Reg domain pending configuration */ >> - u32 reg_ch_conf_pending[2]; >> + DECLARE_BITMAP(reg_ch_conf_pending, 64); >> >> /* Pointer that holds DMA-friendly block for the mailbox */ >> void *mbox;
| |