* [PATCHv7 0/4] register-field manipulation macros
@ 2016-08-19 16:44 Jakub Kicinski
2016-08-19 16:44 ` [PATCHv7 1/4] add basic " Jakub Kicinski
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Jakub Kicinski @ 2016-08-19 16:44 UTC (permalink / raw)
To: Kalle Valo, Linux Wireless List; +Cc: Jakub Kicinski
Hi!
Looks like we're good to go :)
I've thrown in two mt7601u cleanups, macros are as approved
by Linus in the RFC. I'm posting this already to give build
bot a head start while we wait for any additional feedback
on LKML.
-- v6 blurb:
This set moves to a global header file macros which I find
very useful and worth popularising. The basic problem is
that since C bitfields are not very dependable accessing
subfields of registers becomes slightly inconvenient.
It is nice to have the necessary mask and shift operations
wrapped in a macro and have that macro compute shift
at compilation time using FFS.
My implementation follows what Felix Fietkau has done in
mt76. Hannes Frederic Sowa suggested more use of standard
Linux/GCC functions. Since the RFC I've also added a
compile-time check to validate that the value passed to
setters fits in the mask.
I attempted the use of static inlines instead of macros
but it makes GCC < 6.0 barf at the BUILD_BUG_ON()s.
I also noticed that forcing arguments to be u32 for inlines
makes the compiler use 32bit arithmetic where it could
get away with 64bit before (on 64bit machines, obviously).
That's a potential performance concern but probably not
a very practical one today. Apart from looking "cleaner"
static inlines would have the advantage that we could #undef
the auxiliary macros at the end of the header.
Build bot caught a build failure with -Os set. AFAICT gcc
did not handle temporary variable I put in the macro
expression too well. I work around that by defining
__BUILD_BUG_ON_NOT_POWER_OF_2 and using it instead of
BUILD_BUG_ON(!tmp || is_power_of_2(tmp)).
I'm planning to use those macros in another driver soon,
Felix may also use them when upstreaming mt76 if he chooses
to do so.
IMHO if accepted it would be best to push these through
Kalle's wireless drivers' tree, since the only existing
user is in drivers/net/wireless/.
v7:
- drop the explicit type marking (u32/u64) - depend on the type
of the mask instead;
- only allow compilation time constant masks;
- barf at "type of register too small to ever match mask" on get;
- rename PUT -> PREP.
v6:
- do a full rename in patch 2;
- CC many people.
v5:
- repost.
v4:
- add documentation in the header.
v3:
- don't use variables in statement expressions;
- use __BUILD_BUG_ON_NOT_POWER_OF_2.
v2:
- change Felix's email address.
Jakub Kicinski (4):
add basic register-field manipulation macros
mt7601u: remove redefinition of GENMASK
mt7601u: remove unnecessary include
mt7601u: use linux/bitfield.h
drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +-
drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++-
drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++--
drivers/net/wireless/mediatek/mt7601u/init.c | 10 +--
drivers/net/wireless/mediatek/mt7601u/mac.c | 38 +++++-----
drivers/net/wireless/mediatek/mt7601u/main.c | 1 -
drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++---
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +-
drivers/net/wireless/mediatek/mt7601u/phy.c | 44 ++++++------
drivers/net/wireless/mediatek/mt7601u/regs.h | 4 --
drivers/net/wireless/mediatek/mt7601u/tx.c | 19 ++---
drivers/net/wireless/mediatek/mt7601u/util.h | 77 --------------------
include/linux/bitfield.h | 93 +++++++++++++++++++++++++
include/linux/bug.h | 3 +
14 files changed, 177 insertions(+), 160 deletions(-)
delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h
create mode 100644 include/linux/bitfield.h
--
1.9.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCHv7 1/4] add basic register-field manipulation macros 2016-08-19 16:44 [PATCHv7 0/4] register-field manipulation macros Jakub Kicinski @ 2016-08-19 16:44 ` Jakub Kicinski 2016-08-19 16:44 ` [PATCHv7 2/4] mt7601u: remove redefinition of GENMASK Jakub Kicinski ` (3 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Jakub Kicinski @ 2016-08-19 16:44 UTC (permalink / raw) To: Kalle Valo, Linux Wireless List; +Cc: Jakub Kicinski Common approach to accessing register fields is to define structures or sets of macros containing mask and shift pair. Operations on the register are then performed as follows: field = (reg >> shift) & mask; reg &= ~(mask << shift); reg |= (field & mask) << shift; Defining shift and mask separately is tedious. Ivo van Doorn came up with an idea of computing them at compilation time based on a single shifted mask (later refined by Felix) which can be used like this: #define REG_FIELD 0x000ff000 field = FIELD_GET(REG_FIELD, reg); reg &= ~REG_FIELD; reg |= FIELD_PREP(REG_FIELD, field); FIELD_{GET,PREP} macros take care of finding out what the appropriate shift is based on compilation time ffs operation. GENMASK can be used to define registers (which is usually less error-prone and easier to match with datasheets). This approach is the most convenient I've seen so to limit code multiplication let's move the macros to a global header file. Attempts to use static inlines instead of macros failed due to false positive triggering of BUILD_BUG_ON()s, especially with GCC < 6.0. Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> --- include/linux/bitfield.h | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/bug.h | 3 ++ 2 files changed, 96 insertions(+) create mode 100644 include/linux/bitfield.h diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h new file mode 100644 index 000000000000..32ca8863e66d --- /dev/null +++ b/include/linux/bitfield.h @@ -0,0 +1,93 @@ +/* + * Copyright (C) 2014 Felix Fietkau <nbd@nbd.name> + * Copyright (C) 2004 - 2009 Ivo van Doorn <IvDoorn@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 + * as published by the Free Software Foundation + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _LINUX_BITFIELD_H +#define _LINUX_BITFIELD_H + +#include <linux/bug.h> + +/* + * Bitfield access macros + * + * FIELD_{GET,PREP} macros take as first parameter shifted mask + * from which they extract the base mask and shift amount. + * Mask must be a compilation time constant. + * + * Example: + * + * #define REG_FIELD_A GENMASK(6, 0) + * #define REG_FIELD_B BIT(7) + * #define REG_FIELD_C GENMASK(15, 8) + * #define REG_FIELD_D GENMASK(31, 16) + * + * Get: + * a = FIELD_GET(REG_FIELD_A, reg); + * b = FIELD_GET(REG_FIELD_B, reg); + * + * Set: + * reg = FIELD_PREP(REG_FIELD_A, 1) | + * FIELD_PREP(REG_FIELD_B, 0) | + * FIELD_PREP(REG_FIELD_C, c) | + * FIELD_PREP(REG_FIELD_D, 0x40); + * + * Modify: + * reg &= ~REG_FIELD_C; + * reg |= FIELD_PREP(REG_FIELD_C, c); + */ + +#define _bf_shf(x) (__builtin_ffsll(x) - 1) + +#define _BF_FIELD_CHECK(_mask, _reg, _val, _pfx) \ + ({ \ + BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), \ + _pfx "mask is not constant"); \ + BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero"); \ + BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \ + ~((_mask) >> _bf_shf(_mask)) & (_val) : 0, \ + _pfx "value too large for the field"); \ + BUILD_BUG_ON_MSG((_mask) > (typeof(_reg))~0ull, \ + _pfx "type of reg too small for mask"); \ + __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \ + (1ULL << _bf_shf(_mask))); \ + }) + +/** + * FIELD_PREP() - prepare a bitfield element + * @_mask: shifted mask defining the field's length and position + * @_val: value to put in the field + * + * FIELD_PREP() masks and shifts up the value. The result should + * be combined with other fields of the bitfield using logical OR. + */ +#define FIELD_PREP(_mask, _val) \ + ({ \ + _BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \ + ((typeof(_mask))(_val) << _bf_shf(_mask)) & (_mask); \ + }) + +/** + * FIELD_GET() - extract a bitfield element + * @_mask: shifted mask defining the field's length and position + * @_reg: 32bit value of entire bitfield + * + * FIELD_GET() extracts the field specified by @_mask from the + * bitfield passed in as @_reg by masking and shifting it down. + */ +#define FIELD_GET(_mask, _reg) \ + ({ \ + _BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \ + (typeof(_mask))(((_reg) & (_mask)) >> _bf_shf(_mask)); \ + }) + +#endif diff --git a/include/linux/bug.h b/include/linux/bug.h index e51b0709e78d..292d6a10b0c2 100644 --- a/include/linux/bug.h +++ b/include/linux/bug.h @@ -13,6 +13,7 @@ enum bug_trap_type { struct pt_regs; #ifdef __CHECKER__ +#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) (0) #define BUILD_BUG_ON_NOT_POWER_OF_2(n) (0) #define BUILD_BUG_ON_ZERO(e) (0) #define BUILD_BUG_ON_NULL(e) ((void*)0) @@ -24,6 +25,8 @@ struct pt_regs; #else /* __CHECKER__ */ /* Force a compilation error if a constant expression is not a power of 2 */ +#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) \ + BUILD_BUG_ON(((n) & ((n) - 1)) != 0) #define BUILD_BUG_ON_NOT_POWER_OF_2(n) \ BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0)) -- 1.9.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv7 2/4] mt7601u: remove redefinition of GENMASK 2016-08-19 16:44 [PATCHv7 0/4] register-field manipulation macros Jakub Kicinski 2016-08-19 16:44 ` [PATCHv7 1/4] add basic " Jakub Kicinski @ 2016-08-19 16:44 ` Jakub Kicinski 2016-08-19 16:44 ` [PATCHv7 3/4] mt7601u: remove unnecessary include Jakub Kicinski ` (2 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Jakub Kicinski @ 2016-08-19 16:44 UTC (permalink / raw) To: Kalle Valo, Linux Wireless List; +Cc: Jakub Kicinski Remove redefinition of GENMASK which should not be there in the upstream version of the code. Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> --- drivers/net/wireless/mediatek/mt7601u/regs.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/net/wireless/mediatek/mt7601u/regs.h b/drivers/net/wireless/mediatek/mt7601u/regs.h index afd8978e83fa..27a429d90cec 100644 --- a/drivers/net/wireless/mediatek/mt7601u/regs.h +++ b/drivers/net/wireless/mediatek/mt7601u/regs.h @@ -17,10 +17,6 @@ #include <linux/bitops.h> -#ifndef GENMASK -#define GENMASK(h, l) (((U32_C(1) << ((h) - (l) + 1)) - 1) << (l)) -#endif - #define MT_ASIC_VERSION 0x0000 #define MT76XX_REV_E3 0x22 -- 1.9.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv7 3/4] mt7601u: remove unnecessary include 2016-08-19 16:44 [PATCHv7 0/4] register-field manipulation macros Jakub Kicinski 2016-08-19 16:44 ` [PATCHv7 1/4] add basic " Jakub Kicinski 2016-08-19 16:44 ` [PATCHv7 2/4] mt7601u: remove redefinition of GENMASK Jakub Kicinski @ 2016-08-19 16:44 ` Jakub Kicinski 2016-08-19 16:44 ` [PATCHv7 4/4] mt7601u: use linux/bitfield.h Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 0/4] register-field manipulation macros Jakub Kicinski 4 siblings, 0 replies; 16+ messages in thread From: Jakub Kicinski @ 2016-08-19 16:44 UTC (permalink / raw) To: Kalle Valo, Linux Wireless List; +Cc: Jakub Kicinski There is no need to include linux/version.h in a in-tree driver. Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> --- drivers/net/wireless/mediatek/mt7601u/main.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/wireless/mediatek/mt7601u/main.c b/drivers/net/wireless/mediatek/mt7601u/main.c index e70dd9523911..43ebd460ba86 100644 --- a/drivers/net/wireless/mediatek/mt7601u/main.c +++ b/drivers/net/wireless/mediatek/mt7601u/main.c @@ -15,7 +15,6 @@ #include "mt7601u.h" #include "mac.h" #include <linux/etherdevice.h> -#include <linux/version.h> static int mt7601u_start(struct ieee80211_hw *hw) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv7 4/4] mt7601u: use linux/bitfield.h 2016-08-19 16:44 [PATCHv7 0/4] register-field manipulation macros Jakub Kicinski ` (2 preceding siblings ...) 2016-08-19 16:44 ` [PATCHv7 3/4] mt7601u: remove unnecessary include Jakub Kicinski @ 2016-08-19 16:44 ` Jakub Kicinski 2016-08-19 19:02 ` Arend Van Spriel 2016-08-31 11:46 ` [PATCHv8 0/4] register-field manipulation macros Jakub Kicinski 4 siblings, 1 reply; 16+ messages in thread From: Jakub Kicinski @ 2016-08-19 16:44 UTC (permalink / raw) To: Kalle Valo, Linux Wireless List; +Cc: Jakub Kicinski Use the newly added linux/bitfield.h. Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> --- drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +- drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++-- drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++-- drivers/net/wireless/mediatek/mt7601u/init.c | 10 ++-- drivers/net/wireless/mediatek/mt7601u/mac.c | 38 ++++++------ drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++---- drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +- drivers/net/wireless/mediatek/mt7601u/phy.c | 44 +++++++------- drivers/net/wireless/mediatek/mt7601u/tx.c | 19 +++--- drivers/net/wireless/mediatek/mt7601u/util.h | 77 ------------------------- 10 files changed, 81 insertions(+), 155 deletions(-) delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c index 57a80cfa39b1..a8bc064bc14f 100644 --- a/drivers/net/wireless/mediatek/mt7601u/dma.c +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c @@ -103,7 +103,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev, u8 *data, if (unlikely(rxwi->zero[0] || rxwi->zero[1] || rxwi->zero[2])) dev_err_once(dev->dev, "Error: RXWI zero fields are set\n"); - if (unlikely(MT76_GET(MT_RXD_INFO_TYPE, fce_info))) + if (unlikely(FIELD_GET(MT_RXD_INFO_TYPE, fce_info))) dev_err_once(dev->dev, "Error: RX path seen a non-pkt urb\n"); trace_mt_rx(dev, rxwi, fce_info); diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h b/drivers/net/wireless/mediatek/mt7601u/dma.h index 978e8a90b87f..270d126880c0 100644 --- a/drivers/net/wireless/mediatek/mt7601u/dma.h +++ b/drivers/net/wireless/mediatek/mt7601u/dma.h @@ -18,8 +18,6 @@ #include <asm/unaligned.h> #include <linux/skbuff.h> -#include "util.h" - #define MT_DMA_HDR_LEN 4 #define MT_RX_INFO_LEN 4 #define MT_FCE_INFO_LEN 4 @@ -79,9 +77,9 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb, */ info = flags | - MT76_SET(MT_TXD_INFO_LEN, round_up(skb->len, 4)) | - MT76_SET(MT_TXD_INFO_D_PORT, d_port) | - MT76_SET(MT_TXD_INFO_TYPE, type); + FIELD_PREP(MT_TXD_INFO_LEN, round_up(skb->len, 4)) | + FIELD_PREP(MT_TXD_INFO_D_PORT, d_port) | + FIELD_PREP(MT_TXD_INFO_TYPE, type); put_unaligned_le32(info, skb_push(skb, sizeof(info))); return skb_put_padto(skb, round_up(skb->len, 4) + 4); @@ -90,7 +88,7 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb, static inline int mt7601u_dma_skb_wrap_pkt(struct sk_buff *skb, enum mt76_qsel qsel, u32 flags) { - flags |= MT76_SET(MT_TXD_PKT_INFO_QSEL, qsel); + flags |= FIELD_PREP(MT_TXD_PKT_INFO_QSEL, qsel); return mt7601u_dma_skb_wrap(skb, WLAN_PORT, DMA_PACKET, flags); } diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c b/drivers/net/wireless/mediatek/mt7601u/eeprom.c index 8d8ee0344f7b..da6faea092d6 100644 --- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c +++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c @@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, u8 *data, val = mt76_rr(dev, MT_EFUSE_CTRL); val &= ~(MT_EFUSE_CTRL_AIN | MT_EFUSE_CTRL_MODE); - val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) | - MT76_SET(MT_EFUSE_CTRL_MODE, mode) | + val |= FIELD_PREP(MT_EFUSE_CTRL_AIN, addr & ~0xf) | + FIELD_PREP(MT_EFUSE_CTRL_MODE, mode) | MT_EFUSE_CTRL_KICK; mt76_wr(dev, MT_EFUSE_CTRL, val); @@ -128,8 +128,8 @@ mt7601u_set_chip_cap(struct mt7601u_dev *dev, u8 *eeprom) if (!field_valid(nic_conf0 >> 8)) return; - if (MT76_GET(MT_EE_NIC_CONF_0_RX_PATH, nic_conf0) > 1 || - MT76_GET(MT_EE_NIC_CONF_0_TX_PATH, nic_conf0) > 1) + if (FIELD_GET(MT_EE_NIC_CONF_0_RX_PATH, nic_conf0) > 1 || + FIELD_GET(MT_EE_NIC_CONF_0_TX_PATH, nic_conf0) > 1) dev_err(dev->dev, "Error: device has more than 1 RX/TX stream!\n"); } @@ -150,7 +150,7 @@ mt7601u_set_macaddr(struct mt7601u_dev *dev, const u8 *eeprom) mt76_wr(dev, MT_MAC_ADDR_DW0, get_unaligned_le32(dev->macaddr)); mt76_wr(dev, MT_MAC_ADDR_DW1, get_unaligned_le16(dev->macaddr + 4) | - MT76_SET(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff)); + FIELD_PREP(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff)); return 0; } @@ -176,7 +176,7 @@ mt7601u_set_channel_power(struct mt7601u_dev *dev, u8 *eeprom) u8 max_pwr; val = mt7601u_rr(dev, MT_TX_ALC_CFG_0); - max_pwr = MT76_GET(MT_TX_ALC_CFG_0_LIMIT_0, val); + max_pwr = FIELD_GET(MT_TX_ALC_CFG_0_LIMIT_0, val); if (mt7601u_has_tssi(dev, eeprom)) { mt7601u_set_channel_target_power(dev, eeprom, max_pwr); diff --git a/drivers/net/wireless/mediatek/mt7601u/init.c b/drivers/net/wireless/mediatek/mt7601u/init.c index 8fa78d7156be..44d46e25db80 100644 --- a/drivers/net/wireless/mediatek/mt7601u/init.c +++ b/drivers/net/wireless/mediatek/mt7601u/init.c @@ -108,8 +108,9 @@ static void mt7601u_init_usb_dma(struct mt7601u_dev *dev) { u32 val; - val = MT76_SET(MT_USB_DMA_CFG_RX_BULK_AGG_TOUT, MT_USB_AGGR_TIMEOUT) | - MT76_SET(MT_USB_DMA_CFG_RX_BULK_AGG_LMT, MT_USB_AGGR_SIZE_LIMIT) | + val = FIELD_PREP(MT_USB_DMA_CFG_RX_BULK_AGG_TOUT, MT_USB_AGGR_TIMEOUT) | + FIELD_PREP(MT_USB_DMA_CFG_RX_BULK_AGG_LMT, + MT_USB_AGGR_SIZE_LIMIT) | MT_USB_DMA_CFG_RX_BULK_EN | MT_USB_DMA_CFG_TX_BULK_EN; if (dev->in_max_packet == 512) @@ -396,8 +397,9 @@ int mt7601u_init_hardware(struct mt7601u_dev *dev) mt7601u_rmw(dev, MT_US_CYC_CFG, MT_US_CYC_CNT, 0x1e); - mt7601u_wr(dev, MT_TXOP_CTRL_CFG, MT76_SET(MT_TXOP_TRUN_EN, 0x3f) | - MT76_SET(MT_TXOP_EXT_CCA_DLY, 0x58)); + mt7601u_wr(dev, MT_TXOP_CTRL_CFG, + FIELD_PREP(MT_TXOP_TRUN_EN, 0x3f) | + FIELD_PREP(MT_TXOP_EXT_CCA_DLY, 0x58)); ret = mt7601u_eeprom_init(dev); if (ret) diff --git a/drivers/net/wireless/mediatek/mt7601u/mac.c b/drivers/net/wireless/mediatek/mt7601u/mac.c index e21c53ed09fb..3c576392ed89 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mac.c +++ b/drivers/net/wireless/mediatek/mt7601u/mac.c @@ -19,13 +19,13 @@ static void mt76_mac_process_tx_rate(struct ieee80211_tx_rate *txrate, u16 rate) { - u8 idx = MT76_GET(MT_TXWI_RATE_MCS, rate); + u8 idx = FIELD_GET(MT_TXWI_RATE_MCS, rate); txrate->idx = 0; txrate->flags = 0; txrate->count = 1; - switch (MT76_GET(MT_TXWI_RATE_PHY_MODE, rate)) { + switch (FIELD_GET(MT_TXWI_RATE_PHY_MODE, rate)) { case MT_PHY_TYPE_OFDM: txrate->idx = idx + 4; return; @@ -47,7 +47,7 @@ mt76_mac_process_tx_rate(struct ieee80211_tx_rate *txrate, u16 rate) return; } - if (MT76_GET(MT_TXWI_RATE_BW, rate) == MT_PHY_BW_40) + if (FIELD_GET(MT_TXWI_RATE_BW, rate) == MT_PHY_BW_40) txrate->flags |= IEEE80211_TX_RC_40_MHZ_WIDTH; if (rate & MT_TXWI_RATE_SGI) @@ -125,9 +125,9 @@ u16 mt76_mac_tx_rate_val(struct mt7601u_dev *dev, bw = 0; } - rateval = MT76_SET(MT_RXWI_RATE_MCS, rate_idx); - rateval |= MT76_SET(MT_RXWI_RATE_PHY, phy); - rateval |= MT76_SET(MT_RXWI_RATE_BW, bw); + rateval = FIELD_PREP(MT_RXWI_RATE_MCS, rate_idx); + rateval |= FIELD_PREP(MT_RXWI_RATE_PHY, phy); + rateval |= FIELD_PREP(MT_RXWI_RATE_BW, bw); if (rate->flags & IEEE80211_TX_RC_SHORT_GI) rateval |= MT_RXWI_RATE_SGI; @@ -156,9 +156,9 @@ struct mt76_tx_status mt7601u_mac_fetch_tx_status(struct mt7601u_dev *dev) stat.success = !!(val & MT_TX_STAT_FIFO_SUCCESS); stat.aggr = !!(val & MT_TX_STAT_FIFO_AGGR); stat.ack_req = !!(val & MT_TX_STAT_FIFO_ACKREQ); - stat.pktid = MT76_GET(MT_TX_STAT_FIFO_PID_TYPE, val); - stat.wcid = MT76_GET(MT_TX_STAT_FIFO_WCID, val); - stat.rate = MT76_GET(MT_TX_STAT_FIFO_RATE, val); + stat.pktid = FIELD_GET(MT_TX_STAT_FIFO_PID_TYPE, val); + stat.wcid = FIELD_GET(MT_TX_STAT_FIFO_WCID, val); + stat.rate = FIELD_GET(MT_TX_STAT_FIFO_RATE, val); return stat; } @@ -270,7 +270,7 @@ void mt7601u_mac_config_tsf(struct mt7601u_dev *dev, bool enable, int interval) } val &= ~MT_BEACON_TIME_CFG_INTVAL; - val |= MT76_SET(MT_BEACON_TIME_CFG_INTVAL, interval << 4) | + val |= FIELD_PREP(MT_BEACON_TIME_CFG_INTVAL, interval << 4) | MT_BEACON_TIME_CFG_TIMER_EN | MT_BEACON_TIME_CFG_SYNC_MODE | MT_BEACON_TIME_CFG_TBTT_EN; @@ -349,8 +349,8 @@ mt7601u_mac_wcid_setup(struct mt7601u_dev *dev, u8 idx, u8 vif_idx, u8 *mac) u8 zmac[ETH_ALEN] = {}; u32 attr; - attr = MT76_SET(MT_WCID_ATTR_BSS_IDX, vif_idx & 7) | - MT76_SET(MT_WCID_ATTR_BSS_IDX_EXT, !!(vif_idx & 8)); + attr = FIELD_PREP(MT_WCID_ATTR_BSS_IDX, vif_idx & 7) | + FIELD_PREP(MT_WCID_ATTR_BSS_IDX_EXT, !!(vif_idx & 8)); mt76_wr(dev, MT_WCID_ATTR(idx), attr); @@ -382,15 +382,15 @@ void mt7601u_mac_set_ampdu_factor(struct mt7601u_dev *dev) rcu_read_unlock(); mt7601u_wr(dev, MT_MAX_LEN_CFG, 0xa0fff | - MT76_SET(MT_MAX_LEN_CFG_AMPDU, min_factor)); + FIELD_PREP(MT_MAX_LEN_CFG_AMPDU, min_factor)); } static void mt76_mac_process_rate(struct ieee80211_rx_status *status, u16 rate) { - u8 idx = MT76_GET(MT_RXWI_RATE_MCS, rate); + u8 idx = FIELD_GET(MT_RXWI_RATE_MCS, rate); - switch (MT76_GET(MT_RXWI_RATE_PHY, rate)) { + switch (FIELD_GET(MT_RXWI_RATE_PHY, rate)) { case MT_PHY_TYPE_OFDM: if (WARN_ON(idx >= 8)) idx = 0; @@ -436,7 +436,7 @@ mt7601u_rx_monitor_beacon(struct mt7601u_dev *dev, struct mt7601u_rxwi *rxwi, u16 rate, int rssi) { dev->bcn_freq_off = rxwi->freq_off; - dev->bcn_phy_mode = MT76_GET(MT_RXWI_RATE_PHY, rate); + dev->bcn_phy_mode = FIELD_GET(MT_RXWI_RATE_PHY, rate); dev->avg_rssi = (dev->avg_rssi * 15) / 16 + (rssi << 8); } @@ -458,7 +458,7 @@ u32 mt76_mac_process_rx(struct mt7601u_dev *dev, struct sk_buff *skb, u16 rate = le16_to_cpu(rxwi->rate); int rssi; - len = MT76_GET(MT_RXWI_CTL_MPDU_LEN, ctl); + len = FIELD_GET(MT_RXWI_CTL_MPDU_LEN, ctl); if (len < 10) return 0; @@ -542,8 +542,8 @@ int mt76_mac_wcid_set_key(struct mt7601u_dev *dev, u8 idx, val = mt7601u_rr(dev, MT_WCID_ATTR(idx)); val &= ~MT_WCID_ATTR_PKEY_MODE & ~MT_WCID_ATTR_PKEY_MODE_EXT; - val |= MT76_SET(MT_WCID_ATTR_PKEY_MODE, cipher & 7) | - MT76_SET(MT_WCID_ATTR_PKEY_MODE_EXT, cipher >> 3); + val |= FIELD_PREP(MT_WCID_ATTR_PKEY_MODE, cipher & 7) | + FIELD_PREP(MT_WCID_ATTR_PKEY_MODE_EXT, cipher >> 3); val &= ~MT_WCID_ATTR_PAIRWISE; val |= MT_WCID_ATTR_PAIRWISE * !!(key && key->flags & IEEE80211_KEY_FLAG_PAIRWISE); diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c b/drivers/net/wireless/mediatek/mt7601u/mcu.c index 91c4b3427965..dbdfb3f5c507 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c @@ -43,8 +43,8 @@ static inline void mt7601u_dma_skb_wrap_cmd(struct sk_buff *skb, u8 seq, enum mcu_cmd cmd) { WARN_ON(mt7601u_dma_skb_wrap(skb, CPU_TX_PORT, DMA_COMMAND, - MT76_SET(MT_TXD_CMD_INFO_SEQ, seq) | - MT76_SET(MT_TXD_CMD_INFO_TYPE, cmd))); + FIELD_PREP(MT_TXD_CMD_INFO_SEQ, seq) | + FIELD_PREP(MT_TXD_CMD_INFO_TYPE, cmd))); } static inline void trace_mt_mcu_msg_send_cs(struct mt7601u_dev *dev, @@ -100,13 +100,13 @@ static int mt7601u_mcu_wait_resp(struct mt7601u_dev *dev, u8 seq) dev_err(dev->dev, "Error: MCU resp urb failed:%d\n", urb_status); - if (MT76_GET(MT_RXD_CMD_INFO_CMD_SEQ, rxfce) == seq && - MT76_GET(MT_RXD_CMD_INFO_EVT_TYPE, rxfce) == CMD_DONE) + if (FIELD_GET(MT_RXD_CMD_INFO_CMD_SEQ, rxfce) == seq && + FIELD_GET(MT_RXD_CMD_INFO_EVT_TYPE, rxfce) == CMD_DONE) return 0; - dev_err(dev->dev, "Error: MCU resp evt:%hhx seq:%hhx-%hhx!\n", - MT76_GET(MT_RXD_CMD_INFO_EVT_TYPE, rxfce), - seq, MT76_GET(MT_RXD_CMD_INFO_CMD_SEQ, rxfce)); + dev_err(dev->dev, "Error: MCU resp evt:%lx seq:%hhx-%lx!\n", + FIELD_GET(MT_RXD_CMD_INFO_EVT_TYPE, rxfce), + seq, FIELD_GET(MT_RXD_CMD_INFO_CMD_SEQ, rxfce)); } dev_err(dev->dev, "Error: %s timed out\n", __func__); @@ -291,9 +291,9 @@ static int __mt7601u_dma_fw(struct mt7601u_dev *dev, u32 val; int ret; - reg = cpu_to_le32(MT76_SET(MT_TXD_INFO_TYPE, DMA_PACKET) | - MT76_SET(MT_TXD_INFO_D_PORT, CPU_TX_PORT) | - MT76_SET(MT_TXD_INFO_LEN, len)); + reg = cpu_to_le32(FIELD_PREP(MT_TXD_INFO_TYPE, DMA_PACKET) | + FIELD_PREP(MT_TXD_INFO_D_PORT, CPU_TX_PORT) | + FIELD_PREP(MT_TXD_INFO_LEN, len)); memcpy(buf.buf, ®, sizeof(reg)); memcpy(buf.buf + sizeof(reg), data, len); memset(buf.buf + sizeof(reg) + len, 0, 8); diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h index 428bd2f10b7b..c7ec40475a5f 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h +++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h @@ -15,6 +15,7 @@ #ifndef MT7601U_H #define MT7601U_H +#include <linux/bitfield.h> #include <linux/kernel.h> #include <linux/device.h> #include <linux/mutex.h> @@ -24,7 +25,6 @@ #include <linux/debugfs.h> #include "regs.h" -#include "util.h" #define MT_CALIBRATE_INTERVAL (4 * HZ) @@ -299,7 +299,7 @@ bool mt76_poll_msec(struct mt7601u_dev *dev, u32 offset, u32 mask, u32 val, /* Compatibility with mt76 */ #define mt76_rmw_field(_dev, _reg, _field, _val) \ - mt76_rmw(_dev, _reg, _field, MT76_SET(_field, _val)) + mt76_rmw(_dev, _reg, _field, FIELD_PREP(_field, _val)) static inline u32 mt76_rr(struct mt7601u_dev *dev, u32 offset) { diff --git a/drivers/net/wireless/mediatek/mt7601u/phy.c b/drivers/net/wireless/mediatek/mt7601u/phy.c index 1908af6add87..ca09a5d4305e 100644 --- a/drivers/net/wireless/mediatek/mt7601u/phy.c +++ b/drivers/net/wireless/mediatek/mt7601u/phy.c @@ -41,11 +41,12 @@ mt7601u_rf_wr(struct mt7601u_dev *dev, u8 bank, u8 offset, u8 value) goto out; } - mt7601u_wr(dev, MT_RF_CSR_CFG, MT76_SET(MT_RF_CSR_CFG_DATA, value) | - MT76_SET(MT_RF_CSR_CFG_REG_BANK, bank) | - MT76_SET(MT_RF_CSR_CFG_REG_ID, offset) | - MT_RF_CSR_CFG_WR | - MT_RF_CSR_CFG_KICK); + mt7601u_wr(dev, MT_RF_CSR_CFG, + FIELD_PREP(MT_RF_CSR_CFG_DATA, value) | + FIELD_PREP(MT_RF_CSR_CFG_REG_BANK, bank) | + FIELD_PREP(MT_RF_CSR_CFG_REG_ID, offset) | + MT_RF_CSR_CFG_WR | + MT_RF_CSR_CFG_KICK); trace_rf_write(dev, bank, offset, value); out: mutex_unlock(&dev->reg_atomic_mutex); @@ -74,17 +75,18 @@ mt7601u_rf_rr(struct mt7601u_dev *dev, u8 bank, u8 offset) if (!mt76_poll(dev, MT_RF_CSR_CFG, MT_RF_CSR_CFG_KICK, 0, 100)) goto out; - mt7601u_wr(dev, MT_RF_CSR_CFG, MT76_SET(MT_RF_CSR_CFG_REG_BANK, bank) | - MT76_SET(MT_RF_CSR_CFG_REG_ID, offset) | - MT_RF_CSR_CFG_KICK); + mt7601u_wr(dev, MT_RF_CSR_CFG, + FIELD_PREP(MT_RF_CSR_CFG_REG_BANK, bank) | + FIELD_PREP(MT_RF_CSR_CFG_REG_ID, offset) | + MT_RF_CSR_CFG_KICK); if (!mt76_poll(dev, MT_RF_CSR_CFG, MT_RF_CSR_CFG_KICK, 0, 100)) goto out; val = mt7601u_rr(dev, MT_RF_CSR_CFG); - if (MT76_GET(MT_RF_CSR_CFG_REG_ID, val) == offset && - MT76_GET(MT_RF_CSR_CFG_REG_BANK, val) == bank) { - ret = MT76_GET(MT_RF_CSR_CFG_DATA, val); + if (FIELD_GET(MT_RF_CSR_CFG_REG_ID, val) == offset && + FIELD_GET(MT_RF_CSR_CFG_REG_BANK, val) == bank) { + ret = FIELD_GET(MT_RF_CSR_CFG_DATA, val); trace_rf_read(dev, bank, offset, ret); } out: @@ -139,8 +141,8 @@ static void mt7601u_bbp_wr(struct mt7601u_dev *dev, u8 offset, u8 val) } mt7601u_wr(dev, MT_BBP_CSR_CFG, - MT76_SET(MT_BBP_CSR_CFG_VAL, val) | - MT76_SET(MT_BBP_CSR_CFG_REG_NUM, offset) | + FIELD_PREP(MT_BBP_CSR_CFG_VAL, val) | + FIELD_PREP(MT_BBP_CSR_CFG_REG_NUM, offset) | MT_BBP_CSR_CFG_RW_MODE | MT_BBP_CSR_CFG_BUSY); trace_bbp_write(dev, offset, val); out: @@ -163,7 +165,7 @@ static int mt7601u_bbp_rr(struct mt7601u_dev *dev, u8 offset) goto out; mt7601u_wr(dev, MT_BBP_CSR_CFG, - MT76_SET(MT_BBP_CSR_CFG_REG_NUM, offset) | + FIELD_PREP(MT_BBP_CSR_CFG_REG_NUM, offset) | MT_BBP_CSR_CFG_RW_MODE | MT_BBP_CSR_CFG_BUSY | MT_BBP_CSR_CFG_READ); @@ -171,8 +173,8 @@ static int mt7601u_bbp_rr(struct mt7601u_dev *dev, u8 offset) goto out; val = mt7601u_rr(dev, MT_BBP_CSR_CFG); - if (MT76_GET(MT_BBP_CSR_CFG_REG_NUM, val) == offset) { - ret = MT76_GET(MT_BBP_CSR_CFG_VAL, val); + if (FIELD_GET(MT_BBP_CSR_CFG_REG_NUM, val) == offset) { + ret = FIELD_GET(MT_BBP_CSR_CFG_VAL, val); trace_bbp_read(dev, offset, ret); } out: @@ -249,9 +251,9 @@ int mt7601u_phy_get_rssi(struct mt7601u_dev *dev, /* bw40 */ { -2, 16, 34 } } }; - int bw = MT76_GET(MT_RXWI_RATE_BW, rate); - int aux_lna = MT76_GET(MT_RXWI_ANT_AUX_LNA, rxwi->ant); - int lna_id = MT76_GET(MT_RXWI_GAIN_RSSI_LNA_ID, rxwi->gain); + int bw = FIELD_GET(MT_RXWI_RATE_BW, rate); + int aux_lna = FIELD_GET(MT_RXWI_ANT_AUX_LNA, rxwi->ant); + int lna_id = FIELD_GET(MT_RXWI_GAIN_RSSI_LNA_ID, rxwi->gain); int val; if (lna_id) /* LNA id can be 0, 2, 3. */ @@ -259,7 +261,7 @@ int mt7601u_phy_get_rssi(struct mt7601u_dev *dev, val = 8; val -= lna[aux_lna][bw][lna_id]; - val -= MT76_GET(MT_RXWI_GAIN_RSSI_VAL, rxwi->gain); + val -= FIELD_GET(MT_RXWI_GAIN_RSSI_VAL, rxwi->gain); val -= dev->ee->lna_gain; val -= dev->ee->rssi_offset[0]; @@ -939,7 +941,7 @@ static int mt7601u_tssi_cal(struct mt7601u_dev *dev) dev_dbg(dev->dev, "final diff: %08x\n", diff_pwr); val = mt7601u_rr(dev, MT_TX_ALC_CFG_1); - curr_pwr = s6_to_int(MT76_GET(MT_TX_ALC_CFG_1_TEMP_COMP, val)); + curr_pwr = s6_to_int(FIELD_GET(MT_TX_ALC_CFG_1_TEMP_COMP, val)); diff_pwr += curr_pwr; val = (val & ~MT_TX_ALC_CFG_1_TEMP_COMP) | int_to_s6(diff_pwr); mt7601u_wr(dev, MT_TX_ALC_CFG_1, val); diff --git a/drivers/net/wireless/mediatek/mt7601u/tx.c b/drivers/net/wireless/mediatek/mt7601u/tx.c index a0a33dc8f6bc..ad77bec1ba0f 100644 --- a/drivers/net/wireless/mediatek/mt7601u/tx.c +++ b/drivers/net/wireless/mediatek/mt7601u/tx.c @@ -175,11 +175,12 @@ mt7601u_push_txwi(struct mt7601u_dev *dev, struct sk_buff *skb, ba_size = min_t(int, 63, ba_size); if (info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE) ba_size = 0; - txwi->ack_ctl |= MT76_SET(MT_TXWI_ACK_CTL_BA_WINDOW, ba_size); + txwi->ack_ctl |= FIELD_PREP(MT_TXWI_ACK_CTL_BA_WINDOW, ba_size); - txwi->flags = cpu_to_le16(MT_TXWI_FLAGS_AMPDU | - MT76_SET(MT_TXWI_FLAGS_MPDU_DENSITY, - sta->ht_cap.ampdu_density)); + txwi->flags = + cpu_to_le16(MT_TXWI_FLAGS_AMPDU | + FIELD_PREP(MT_TXWI_FLAGS_MPDU_DENSITY, + sta->ht_cap.ampdu_density)); if (info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE) txwi->flags = 0; } @@ -188,7 +189,7 @@ mt7601u_push_txwi(struct mt7601u_dev *dev, struct sk_buff *skb, is_probe = !!(info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE); pkt_id = mt7601u_tx_pktid_enc(dev, rate_ctl & 0x7, is_probe); - pkt_len |= MT76_SET(MT_TXWI_LEN_PKTID, pkt_id); + pkt_len |= FIELD_PREP(MT_TXWI_LEN_PKTID, pkt_id); txwi->len_ctl = cpu_to_le16(pkt_len); return txwi; @@ -285,9 +286,9 @@ int mt7601u_conf_tx(struct ieee80211_hw *hw, struct ieee80211_vif *vif, WARN_ON(cw_min > 0xf); WARN_ON(cw_max > 0xf); - val = MT76_SET(MT_EDCA_CFG_AIFSN, params->aifs) | - MT76_SET(MT_EDCA_CFG_CWMIN, cw_min) | - MT76_SET(MT_EDCA_CFG_CWMAX, cw_max); + val = FIELD_PREP(MT_EDCA_CFG_AIFSN, params->aifs) | + FIELD_PREP(MT_EDCA_CFG_CWMIN, cw_min) | + FIELD_PREP(MT_EDCA_CFG_CWMAX, cw_max); /* TODO: based on user-controlled EnableTxBurst var vendor drv sets * a really long txop on AC0 (see connect.c:2009) but only on * connect? When not connected should be 0. @@ -295,7 +296,7 @@ int mt7601u_conf_tx(struct ieee80211_hw *hw, struct ieee80211_vif *vif, if (!hw_q) val |= 0x60; else - val |= MT76_SET(MT_EDCA_CFG_TXOP, params->txop); + val |= FIELD_PREP(MT_EDCA_CFG_TXOP, params->txop); mt76_wr(dev, MT_EDCA_CFG_AC(hw_q), val); val = mt76_rr(dev, MT_WMM_TXOP(hw_q)); diff --git a/drivers/net/wireless/mediatek/mt7601u/util.h b/drivers/net/wireless/mediatek/mt7601u/util.h deleted file mode 100644 index b89140bf1210..000000000000 --- a/drivers/net/wireless/mediatek/mt7601u/util.h +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright (C) 2014 Felix Fietkau <nbd@openwrt.org> - * Copyright (C) 2004 - 2009 Ivo van Doorn <IvDoorn@gmail.com> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#ifndef __MT76_UTIL_H -#define __MT76_UTIL_H - -/* - * Power of two check, this will check - * if the mask that has been given contains and contiguous set of bits. - * Note that we cannot use the is_power_of_2() function since this - * check must be done at compile-time. - */ -#define is_power_of_two(x) ( !((x) & ((x)-1)) ) -#define low_bit_mask(x) ( ((x)-1) & ~(x) ) -#define is_valid_mask(x) is_power_of_two(1LU + (x) + low_bit_mask(x)) - -/* - * Macros to find first set bit in a variable. - * These macros behave the same as the __ffs() functions but - * the most important difference that this is done during - * compile-time rather then run-time. - */ -#define compile_ffs2(__x) \ - __builtin_choose_expr(((__x) & 0x1), 0, 1) - -#define compile_ffs4(__x) \ - __builtin_choose_expr(((__x) & 0x3), \ - (compile_ffs2((__x))), \ - (compile_ffs2((__x) >> 2) + 2)) - -#define compile_ffs8(__x) \ - __builtin_choose_expr(((__x) & 0xf), \ - (compile_ffs4((__x))), \ - (compile_ffs4((__x) >> 4) + 4)) - -#define compile_ffs16(__x) \ - __builtin_choose_expr(((__x) & 0xff), \ - (compile_ffs8((__x))), \ - (compile_ffs8((__x) >> 8) + 8)) - -#define compile_ffs32(__x) \ - __builtin_choose_expr(((__x) & 0xffff), \ - (compile_ffs16((__x))), \ - (compile_ffs16((__x) >> 16) + 16)) - -/* - * This macro will check the requirements for the FIELD{8,16,32} macros - * The mask should be a constant non-zero contiguous set of bits which - * does not exceed the given typelimit. - */ -#define FIELD_CHECK(__mask) \ - BUILD_BUG_ON(!(__mask) || !is_valid_mask(__mask)) - -#define MT76_SET(_mask, _val) \ - ({ \ - FIELD_CHECK(_mask); \ - (((u32) (_val)) << compile_ffs32(_mask)) & _mask; \ - }) - -#define MT76_GET(_mask, _val) \ - ({ \ - FIELD_CHECK(_mask); \ - (u32) (((_val) & _mask) >> compile_ffs32(_mask)); \ - }) - -#endif -- 1.9.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCHv7 4/4] mt7601u: use linux/bitfield.h 2016-08-19 16:44 ` [PATCHv7 4/4] mt7601u: use linux/bitfield.h Jakub Kicinski @ 2016-08-19 19:02 ` Arend Van Spriel 2016-08-22 10:52 ` Jakub Kicinski 0 siblings, 1 reply; 16+ messages in thread From: Arend Van Spriel @ 2016-08-19 19:02 UTC (permalink / raw) To: Jakub Kicinski, Kalle Valo, Linux Wireless List On 19-8-2016 18:44, Jakub Kicinski wrote: > Use the newly added linux/bitfield.h. > > Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> > --- > drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +- > drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++-- > drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++-- > drivers/net/wireless/mediatek/mt7601u/init.c | 10 ++-- > drivers/net/wireless/mediatek/mt7601u/mac.c | 38 ++++++------ > drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++---- > drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +- > drivers/net/wireless/mediatek/mt7601u/phy.c | 44 +++++++------- > drivers/net/wireless/mediatek/mt7601u/tx.c | 19 +++--- > drivers/net/wireless/mediatek/mt7601u/util.h | 77 ------------------------- > 10 files changed, 81 insertions(+), 155 deletions(-) > delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h > > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c > index 57a80cfa39b1..a8bc064bc14f 100644 > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c > @@ -103,7 +103,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev, u8 *data, > > if (unlikely(rxwi->zero[0] || rxwi->zero[1] || rxwi->zero[2])) > dev_err_once(dev->dev, "Error: RXWI zero fields are set\n"); > - if (unlikely(MT76_GET(MT_RXD_INFO_TYPE, fce_info))) > + if (unlikely(FIELD_GET(MT_RXD_INFO_TYPE, fce_info))) > dev_err_once(dev->dev, "Error: RX path seen a non-pkt urb\n"); > > trace_mt_rx(dev, rxwi, fce_info); > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h b/drivers/net/wireless/mediatek/mt7601u/dma.h > index 978e8a90b87f..270d126880c0 100644 > --- a/drivers/net/wireless/mediatek/mt7601u/dma.h > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.h > @@ -18,8 +18,6 @@ > #include <asm/unaligned.h> > #include <linux/skbuff.h> > > -#include "util.h" > - > #define MT_DMA_HDR_LEN 4 > #define MT_RX_INFO_LEN 4 > #define MT_FCE_INFO_LEN 4 > @@ -79,9 +77,9 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb, > */ > > info = flags | > - MT76_SET(MT_TXD_INFO_LEN, round_up(skb->len, 4)) | > - MT76_SET(MT_TXD_INFO_D_PORT, d_port) | > - MT76_SET(MT_TXD_INFO_TYPE, type); > + FIELD_PREP(MT_TXD_INFO_LEN, round_up(skb->len, 4)) | > + FIELD_PREP(MT_TXD_INFO_D_PORT, d_port) | > + FIELD_PREP(MT_TXD_INFO_TYPE, type); So what are those flags? Is there no field definition for those. > put_unaligned_le32(info, skb_push(skb, sizeof(info))); > return skb_put_padto(skb, round_up(skb->len, 4) + 4); > @@ -90,7 +88,7 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb, > static inline int > mt7601u_dma_skb_wrap_pkt(struct sk_buff *skb, enum mt76_qsel qsel, u32 flags) > { > - flags |= MT76_SET(MT_TXD_PKT_INFO_QSEL, qsel); > + flags |= FIELD_PREP(MT_TXD_PKT_INFO_QSEL, qsel); Ah. This is the flags being ORed in above. I suppose there are more callsites to mt7601u_dma_skb_wrap(). > return mt7601u_dma_skb_wrap(skb, WLAN_PORT, DMA_PACKET, flags); > } > > diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c b/drivers/net/wireless/mediatek/mt7601u/eeprom.c > index 8d8ee0344f7b..da6faea092d6 100644 > --- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c > +++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c > @@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, u8 *data, > val = mt76_rr(dev, MT_EFUSE_CTRL); > val &= ~(MT_EFUSE_CTRL_AIN | > MT_EFUSE_CTRL_MODE); > - val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) | > - MT76_SET(MT_EFUSE_CTRL_MODE, mode) | > + val |= FIELD_PREP(MT_EFUSE_CTRL_AIN, addr & ~0xf) | > + FIELD_PREP(MT_EFUSE_CTRL_MODE, mode) | > MT_EFUSE_CTRL_KICK; MT_EFUSE_CTRL_KICK is probably a 1-bit field in MT_EFUSE_CTRL register. It looks like you did not want to go all the way although you do give an example in bitfield.h, ie. + * #define REG_FIELD_B BIT(7). Regards, Arend ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv7 4/4] mt7601u: use linux/bitfield.h 2016-08-19 19:02 ` Arend Van Spriel @ 2016-08-22 10:52 ` Jakub Kicinski 2016-08-22 19:19 ` Arend Van Spriel 0 siblings, 1 reply; 16+ messages in thread From: Jakub Kicinski @ 2016-08-22 10:52 UTC (permalink / raw) To: Arend Van Spriel; +Cc: Kalle Valo, Linux Wireless List On Fri, 19 Aug 2016 21:02:02 +0200, Arend Van Spriel wrote: > On 19-8-2016 18:44, Jakub Kicinski wrote: > > Use the newly added linux/bitfield.h. > > > > Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> > > --- > > drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +- > > drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++-- > > drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++-- > > drivers/net/wireless/mediatek/mt7601u/init.c | 10 ++-- > > drivers/net/wireless/mediatek/mt7601u/mac.c | 38 ++++++------ > > drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++---- > > drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +- > > drivers/net/wireless/mediatek/mt7601u/phy.c | 44 +++++++------- > > drivers/net/wireless/mediatek/mt7601u/tx.c | 19 +++--- > > drivers/net/wireless/mediatek/mt7601u/util.h | 77 ------------------------- > > 10 files changed, 81 insertions(+), 155 deletions(-) > > delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h > > > > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c > > index 57a80cfa39b1..a8bc064bc14f 100644 > > --- a/drivers/net/wireless/mediatek/mt7601u/dma.c > > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c > > @@ -103,7 +103,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev, u8 *data, > > > > if (unlikely(rxwi->zero[0] || rxwi->zero[1] || rxwi->zero[2])) > > dev_err_once(dev->dev, "Error: RXWI zero fields are set\n"); > > - if (unlikely(MT76_GET(MT_RXD_INFO_TYPE, fce_info))) > > + if (unlikely(FIELD_GET(MT_RXD_INFO_TYPE, fce_info))) > > dev_err_once(dev->dev, "Error: RX path seen a non-pkt urb\n"); > > > > trace_mt_rx(dev, rxwi, fce_info); > > diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h b/drivers/net/wireless/mediatek/mt7601u/dma.h > > index 978e8a90b87f..270d126880c0 100644 > > --- a/drivers/net/wireless/mediatek/mt7601u/dma.h > > +++ b/drivers/net/wireless/mediatek/mt7601u/dma.h > > @@ -18,8 +18,6 @@ > > #include <asm/unaligned.h> > > #include <linux/skbuff.h> > > > > -#include "util.h" > > - > > #define MT_DMA_HDR_LEN 4 > > #define MT_RX_INFO_LEN 4 > > #define MT_FCE_INFO_LEN 4 > > @@ -79,9 +77,9 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb, > > */ > > > > info = flags | > > - MT76_SET(MT_TXD_INFO_LEN, round_up(skb->len, 4)) | > > - MT76_SET(MT_TXD_INFO_D_PORT, d_port) | > > - MT76_SET(MT_TXD_INFO_TYPE, type); > > + FIELD_PREP(MT_TXD_INFO_LEN, round_up(skb->len, 4)) | > > + FIELD_PREP(MT_TXD_INFO_D_PORT, d_port) | > > + FIELD_PREP(MT_TXD_INFO_TYPE, type); > > So what are those flags? Is there no field definition for those. > > > put_unaligned_le32(info, skb_push(skb, sizeof(info))); > > return skb_put_padto(skb, round_up(skb->len, 4) + 4); > > @@ -90,7 +88,7 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb, > > static inline int > > mt7601u_dma_skb_wrap_pkt(struct sk_buff *skb, enum mt76_qsel qsel, u32 flags) > > { > > - flags |= MT76_SET(MT_TXD_PKT_INFO_QSEL, qsel); > > + flags |= FIELD_PREP(MT_TXD_PKT_INFO_QSEL, qsel); > > Ah. This is the flags being ORed in above. I suppose there are more > callsites to mt7601u_dma_skb_wrap(). Yes, the field layout differs slightly between data and MCU commands. Additionally some packet flags depend on mac80211 info and I didn't want to pollute dma.h with mac80211 info so I just pass those flags as opaque u32. > > return mt7601u_dma_skb_wrap(skb, WLAN_PORT, DMA_PACKET, flags); > > } > > > > diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c b/drivers/net/wireless/mediatek/mt7601u/eeprom.c > > index 8d8ee0344f7b..da6faea092d6 100644 > > --- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c > > +++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c > > @@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, u8 *data, > > val = mt76_rr(dev, MT_EFUSE_CTRL); > > val &= ~(MT_EFUSE_CTRL_AIN | > > MT_EFUSE_CTRL_MODE); > > - val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) | > > - MT76_SET(MT_EFUSE_CTRL_MODE, mode) | > > + val |= FIELD_PREP(MT_EFUSE_CTRL_AIN, addr & ~0xf) | > > + FIELD_PREP(MT_EFUSE_CTRL_MODE, mode) | > > MT_EFUSE_CTRL_KICK; > > MT_EFUSE_CTRL_KICK is probably a 1-bit field in MT_EFUSE_CTRL register. > It looks like you did not want to go all the way although you do give an > example in bitfield.h, ie. + * #define REG_FIELD_B BIT(7). True, I just wanted to show in the examples that BIT() is OK to use. My feeling is that ORing in always set flags is acceptable, or at least it was my feeling when I wrote this code ;) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv7 4/4] mt7601u: use linux/bitfield.h 2016-08-22 10:52 ` Jakub Kicinski @ 2016-08-22 19:19 ` Arend Van Spriel 2016-08-22 20:59 ` Jakub Kicinski 0 siblings, 1 reply; 16+ messages in thread From: Arend Van Spriel @ 2016-08-22 19:19 UTC (permalink / raw) To: Jakub Kicinski; +Cc: Kalle Valo, Linux Wireless List On 22-8-2016 12:52, Jakub Kicinski wrote: > On Fri, 19 Aug 2016 21:02:02 +0200, Arend Van Spriel wrote: >> On 19-8-2016 18:44, Jakub Kicinski wrote: >>> Use the newly added linux/bitfield.h. >>> >>> Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> >>> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> >>> --- [snip] >>> diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c b/drivers/net/wireless/mediatek/mt7601u/eeprom.c >>> index 8d8ee0344f7b..da6faea092d6 100644 >>> --- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c >>> +++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c >>> @@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, u8 *data, >>> val = mt76_rr(dev, MT_EFUSE_CTRL); >>> val &= ~(MT_EFUSE_CTRL_AIN | >>> MT_EFUSE_CTRL_MODE); >>> - val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) | >>> - MT76_SET(MT_EFUSE_CTRL_MODE, mode) | >>> + val |= FIELD_PREP(MT_EFUSE_CTRL_AIN, addr & ~0xf) | >>> + FIELD_PREP(MT_EFUSE_CTRL_MODE, mode) | >>> MT_EFUSE_CTRL_KICK; >> >> MT_EFUSE_CTRL_KICK is probably a 1-bit field in MT_EFUSE_CTRL register. >> It looks like you did not want to go all the way although you do give an >> example in bitfield.h, ie. + * #define REG_FIELD_B BIT(7). > > True, I just wanted to show in the examples that BIT() is OK to use. > My feeling is that ORing in always set flags is acceptable, or at least > it was my feeling when I wrote this code ;) I find it tricky to have the user do the field clearing separately. Especially if you allow the calling function to pass additional opaque "flags". In my experience this is more error prone than anything else when dealing with bit fields and as such it is unfortunate this aspect is not addressed in your patches. I would rather see: val = mt76_rr(dev, MT_EFUSE_CTRL); FIELD_SET(val, MT_EFUSE_CTRL_AIN, addr & ~0xf); FIELD_SET(val, MT_EFUSE_CTRL_MODE, mode); FIELD_SET(val, MT_EFUSE_CTRL_KICK, 1); mt76_wr(dev, MT_EFUSE_CTRL, val); in which FIELD_SET takes care of clearing the indicated field. I am wondering how much these two solutions differ in terms of assembly instructions. Regards, Arend ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv7 4/4] mt7601u: use linux/bitfield.h 2016-08-22 19:19 ` Arend Van Spriel @ 2016-08-22 20:59 ` Jakub Kicinski 0 siblings, 0 replies; 16+ messages in thread From: Jakub Kicinski @ 2016-08-22 20:59 UTC (permalink / raw) To: Arend Van Spriel; +Cc: Kalle Valo, Linux Wireless List On Mon, 22 Aug 2016 21:19:41 +0200, Arend Van Spriel wrote: > On 22-8-2016 12:52, Jakub Kicinski wrote: > > On Fri, 19 Aug 2016 21:02:02 +0200, Arend Van Spriel wrote: > >> On 19-8-2016 18:44, Jakub Kicinski wrote: > >>> Use the newly added linux/bitfield.h. > >>> > >>> Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> > >>> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> > >>> --- > > [snip] > > >>> diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c b/drivers/net/wireless/mediatek/mt7601u/eeprom.c > >>> index 8d8ee0344f7b..da6faea092d6 100644 > >>> --- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c > >>> +++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c > >>> @@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, u8 *data, > >>> val = mt76_rr(dev, MT_EFUSE_CTRL); > >>> val &= ~(MT_EFUSE_CTRL_AIN | > >>> MT_EFUSE_CTRL_MODE); > >>> - val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) | > >>> - MT76_SET(MT_EFUSE_CTRL_MODE, mode) | > >>> + val |= FIELD_PREP(MT_EFUSE_CTRL_AIN, addr & ~0xf) | > >>> + FIELD_PREP(MT_EFUSE_CTRL_MODE, mode) | > >>> MT_EFUSE_CTRL_KICK; > >> > >> MT_EFUSE_CTRL_KICK is probably a 1-bit field in MT_EFUSE_CTRL register. > >> It looks like you did not want to go all the way although you do give an > >> example in bitfield.h, ie. + * #define REG_FIELD_B BIT(7). > > > > True, I just wanted to show in the examples that BIT() is OK to use. > > My feeling is that ORing in always set flags is acceptable, or at least > > it was my feeling when I wrote this code ;) > > I find it tricky to have the user do the field clearing separately. > Especially if you allow the calling function to pass additional opaque > "flags". In my experience this is more error prone than anything else > when dealing with bit fields and as such it is unfortunate this aspect > is not addressed in your patches. I don't think anything in this set prevents people from adding a FIELD_SET() wrapper. Quite the opposite and I'd encourage it. > I would rather see: > > val = mt76_rr(dev, MT_EFUSE_CTRL); > FIELD_SET(val, MT_EFUSE_CTRL_AIN, addr & ~0xf); > FIELD_SET(val, MT_EFUSE_CTRL_MODE, mode); > FIELD_SET(val, MT_EFUSE_CTRL_KICK, 1); > mt76_wr(dev, MT_EFUSE_CTRL, val); > > in which FIELD_SET takes care of clearing the indicated field. Yes, I agree this is a good solution as well. The reason I didn't go with this approach (apart from modifying an argument of a macro ;)) is the experience with rt2x00 which followed this design and I wasn't particularly fond of the resulting code. I find PREP macro easier to read (less parameters). It's also often convenient to not have to zero the variable for pure write and be able to combine the values in a parameter list: + mt7601u_wr(dev, MT_TXOP_CTRL_CFG, + FIELD_PREP(MT_TXOP_TRUN_EN, 0x3f) | + FIELD_PREP(MT_TXOP_EXT_CCA_DLY, 0x58)); or: + reg = cpu_to_le32(FIELD_PREP(MT_TXD_INFO_TYPE, DMA_PACKET) | + FIELD_PREP(MT_TXD_INFO_D_PORT, CPU_TX_PORT) | + FIELD_PREP(MT_TXD_INFO_LEN, len)); So each approach has it's advantages and I think they complement each other. Please note that I'm just using mt7601u as an example, I really don't need SET in the new code I'm trying to use these macros for now [1]. [1] https://patchwork.ozlabs.org/patch/628779/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv8 0/4] register-field manipulation macros 2016-08-19 16:44 [PATCHv7 0/4] register-field manipulation macros Jakub Kicinski ` (3 preceding siblings ...) 2016-08-19 16:44 ` [PATCHv7 4/4] mt7601u: use linux/bitfield.h Jakub Kicinski @ 2016-08-31 11:46 ` Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 1/4] add basic " Jakub Kicinski ` (4 more replies) 4 siblings, 5 replies; 16+ messages in thread From: Jakub Kicinski @ 2016-08-31 11:46 UTC (permalink / raw) To: Kalle Valo, Linux Wireless List; +Cc: Jakub Kicinski Hi! Small improvement suggested by Daniel Borkmann - use two underscore prefix. https://www.mail-archive.com/netdev@vger.kernel.org/msg125423.html -- v6 blurb: This set moves to a global header file macros which I find very useful and worth popularising. The basic problem is that since C bitfields are not very dependable accessing subfields of registers becomes slightly inconvenient. It is nice to have the necessary mask and shift operations wrapped in a macro and have that macro compute shift at compilation time using FFS. My implementation follows what Felix Fietkau has done in mt76. Hannes Frederic Sowa suggested more use of standard Linux/GCC functions. Since the RFC I've also added a compile-time check to validate that the value passed to setters fits in the mask. I attempted the use of static inlines instead of macros but it makes GCC < 6.0 barf at the BUILD_BUG_ON()s. I also noticed that forcing arguments to be u32 for inlines makes the compiler use 32bit arithmetic where it could get away with 64bit before (on 64bit machines, obviously). That's a potential performance concern but probably not a very practical one today. Apart from looking "cleaner" static inlines would have the advantage that we could #undef the auxiliary macros at the end of the header. Build bot caught a build failure with -Os set. AFAICT gcc did not handle temporary variable I put in the macro expression too well. I work around that by defining __BUILD_BUG_ON_NOT_POWER_OF_2 and using it instead of BUILD_BUG_ON(!tmp || is_power_of_2(tmp)). I'm planning to use those macros in another driver soon, Felix may also use them when upstreaming mt76 if he chooses to do so. IMHO if accepted it would be best to push these through Kalle's wireless drivers' tree, since the only existing user is in drivers/net/wireless/. v8: - use two underscores for auxiliary macros (Daniel B). v7: - drop the explicit type marking (u32/u64) - depend on the type of the mask instead; - only allow compilation time constant masks; - barf at "type of register too small to ever match mask" on get; - rename PUT -> PREP. v6: - do a full rename in patch 2; - CC many people. v5: - repost. v4: - add documentation in the header. v3: - don't use variables in statement expressions; - use __BUILD_BUG_ON_NOT_POWER_OF_2. v2: - change Felix's email address. Jakub Kicinski (4): add basic register-field manipulation macros mt7601u: remove redefinition of GENMASK mt7601u: remove unnecessary include mt7601u: use linux/bitfield.h drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +- drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++- drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++-- drivers/net/wireless/mediatek/mt7601u/init.c | 10 +-- drivers/net/wireless/mediatek/mt7601u/mac.c | 38 +++++----- drivers/net/wireless/mediatek/mt7601u/main.c | 1 - drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++--- drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +- drivers/net/wireless/mediatek/mt7601u/phy.c | 44 ++++++------ drivers/net/wireless/mediatek/mt7601u/regs.h | 4 -- drivers/net/wireless/mediatek/mt7601u/tx.c | 19 ++--- drivers/net/wireless/mediatek/mt7601u/util.h | 77 -------------------- include/linux/bitfield.h | 93 +++++++++++++++++++++++++ include/linux/bug.h | 3 + 14 files changed, 177 insertions(+), 160 deletions(-) delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h create mode 100644 include/linux/bitfield.h -- 1.9.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv8 1/4] add basic register-field manipulation macros 2016-08-31 11:46 ` [PATCHv8 0/4] register-field manipulation macros Jakub Kicinski @ 2016-08-31 11:46 ` Jakub Kicinski 2016-09-09 9:11 ` [PATCHv8,1/4] " Kalle Valo 2016-08-31 11:46 ` [PATCHv8 2/4] mt7601u: remove redefinition of GENMASK Jakub Kicinski ` (3 subsequent siblings) 4 siblings, 1 reply; 16+ messages in thread From: Jakub Kicinski @ 2016-08-31 11:46 UTC (permalink / raw) To: Kalle Valo, Linux Wireless List; +Cc: Jakub Kicinski Common approach to accessing register fields is to define structures or sets of macros containing mask and shift pair. Operations on the register are then performed as follows: field = (reg >> shift) & mask; reg &= ~(mask << shift); reg |= (field & mask) << shift; Defining shift and mask separately is tedious. Ivo van Doorn came up with an idea of computing them at compilation time based on a single shifted mask (later refined by Felix) which can be used like this: #define REG_FIELD 0x000ff000 field = FIELD_GET(REG_FIELD, reg); reg &= ~REG_FIELD; reg |= FIELD_PREP(REG_FIELD, field); FIELD_{GET,PREP} macros take care of finding out what the appropriate shift is based on compilation time ffs operation. GENMASK can be used to define registers (which is usually less error-prone and easier to match with datasheets). This approach is the most convenient I've seen so to limit code multiplication let's move the macros to a global header file. Attempts to use static inlines instead of macros failed due to false positive triggering of BUILD_BUG_ON()s, especially with GCC < 6.0. Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> --- v8: - use two underscores as prefix for __bf_shf() and __BF_FIELD_CHECK() include/linux/bitfield.h | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/bug.h | 3 ++ 2 files changed, 96 insertions(+) create mode 100644 include/linux/bitfield.h diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h new file mode 100644 index 000000000000..f6505d83069d --- /dev/null +++ b/include/linux/bitfield.h @@ -0,0 +1,93 @@ +/* + * Copyright (C) 2014 Felix Fietkau <nbd@nbd.name> + * Copyright (C) 2004 - 2009 Ivo van Doorn <IvDoorn@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 + * as published by the Free Software Foundation + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _LINUX_BITFIELD_H +#define _LINUX_BITFIELD_H + +#include <linux/bug.h> + +/* + * Bitfield access macros + * + * FIELD_{GET,PREP} macros take as first parameter shifted mask + * from which they extract the base mask and shift amount. + * Mask must be a compilation time constant. + * + * Example: + * + * #define REG_FIELD_A GENMASK(6, 0) + * #define REG_FIELD_B BIT(7) + * #define REG_FIELD_C GENMASK(15, 8) + * #define REG_FIELD_D GENMASK(31, 16) + * + * Get: + * a = FIELD_GET(REG_FIELD_A, reg); + * b = FIELD_GET(REG_FIELD_B, reg); + * + * Set: + * reg = FIELD_PREP(REG_FIELD_A, 1) | + * FIELD_PREP(REG_FIELD_B, 0) | + * FIELD_PREP(REG_FIELD_C, c) | + * FIELD_PREP(REG_FIELD_D, 0x40); + * + * Modify: + * reg &= ~REG_FIELD_C; + * reg |= FIELD_PREP(REG_FIELD_C, c); + */ + +#define __bf_shf(x) (__builtin_ffsll(x) - 1) + +#define __BF_FIELD_CHECK(_mask, _reg, _val, _pfx) \ + ({ \ + BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask), \ + _pfx "mask is not constant"); \ + BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero"); \ + BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \ + ~((_mask) >> __bf_shf(_mask)) & (_val) : 0, \ + _pfx "value too large for the field"); \ + BUILD_BUG_ON_MSG((_mask) > (typeof(_reg))~0ull, \ + _pfx "type of reg too small for mask"); \ + __BUILD_BUG_ON_NOT_POWER_OF_2((_mask) + \ + (1ULL << __bf_shf(_mask))); \ + }) + +/** + * FIELD_PREP() - prepare a bitfield element + * @_mask: shifted mask defining the field's length and position + * @_val: value to put in the field + * + * FIELD_PREP() masks and shifts up the value. The result should + * be combined with other fields of the bitfield using logical OR. + */ +#define FIELD_PREP(_mask, _val) \ + ({ \ + __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \ + ((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask); \ + }) + +/** + * FIELD_GET() - extract a bitfield element + * @_mask: shifted mask defining the field's length and position + * @_reg: 32bit value of entire bitfield + * + * FIELD_GET() extracts the field specified by @_mask from the + * bitfield passed in as @_reg by masking and shifting it down. + */ +#define FIELD_GET(_mask, _reg) \ + ({ \ + __BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \ + (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask)); \ + }) + +#endif diff --git a/include/linux/bug.h b/include/linux/bug.h index e51b0709e78d..292d6a10b0c2 100644 --- a/include/linux/bug.h +++ b/include/linux/bug.h @@ -13,6 +13,7 @@ enum bug_trap_type { struct pt_regs; #ifdef __CHECKER__ +#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) (0) #define BUILD_BUG_ON_NOT_POWER_OF_2(n) (0) #define BUILD_BUG_ON_ZERO(e) (0) #define BUILD_BUG_ON_NULL(e) ((void*)0) @@ -24,6 +25,8 @@ struct pt_regs; #else /* __CHECKER__ */ /* Force a compilation error if a constant expression is not a power of 2 */ +#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) \ + BUILD_BUG_ON(((n) & ((n) - 1)) != 0) #define BUILD_BUG_ON_NOT_POWER_OF_2(n) \ BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0)) -- 1.9.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCHv8,1/4] add basic register-field manipulation macros 2016-08-31 11:46 ` [PATCHv8 1/4] add basic " Jakub Kicinski @ 2016-09-09 9:11 ` Kalle Valo 0 siblings, 0 replies; 16+ messages in thread From: Kalle Valo @ 2016-09-09 9:11 UTC (permalink / raw) To: Jakub Kicinski; +Cc: Linux Wireless List, Jakub Kicinski Jakub Kicinski <jakub.kicinski@netronome.com> wrote: > Common approach to accessing register fields is to define > structures or sets of macros containing mask and shift pair. > Operations on the register are then performed as follows: > > field = (reg >> shift) & mask; > > reg &= ~(mask << shift); > reg |= (field & mask) << shift; > > Defining shift and mask separately is tedious. Ivo van Doorn > came up with an idea of computing them at compilation time > based on a single shifted mask (later refined by Felix) which > can be used like this: > > #define REG_FIELD 0x000ff000 > > field = FIELD_GET(REG_FIELD, reg); > > reg &= ~REG_FIELD; > reg |= FIELD_PREP(REG_FIELD, field); > > FIELD_{GET,PREP} macros take care of finding out what the > appropriate shift is based on compilation time ffs operation. > > GENMASK can be used to define registers (which is usually > less error-prone and easier to match with datasheets). > > This approach is the most convenient I've seen so to limit code > multiplication let's move the macros to a global header file. > Attempts to use static inlines instead of macros failed due > to false positive triggering of BUILD_BUG_ON()s, especially with > GCC < 6.0. > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> > Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> Thanks, 4 patches applied to wireless-drivers-next.git: 3e9b3112ec74 add basic register-field manipulation macros faad5433b722 mt7601u: remove redefinition of GENMASK adcc710d0a9e mt7601u: remove unnecessary include d43af50566b4 mt7601u: use linux/bitfield.h -- Sent by pwcli https://patchwork.kernel.org/patch/9306999/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv8 2/4] mt7601u: remove redefinition of GENMASK 2016-08-31 11:46 ` [PATCHv8 0/4] register-field manipulation macros Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 1/4] add basic " Jakub Kicinski @ 2016-08-31 11:46 ` Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 3/4] mt7601u: remove unnecessary include Jakub Kicinski ` (2 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Jakub Kicinski @ 2016-08-31 11:46 UTC (permalink / raw) To: Kalle Valo, Linux Wireless List; +Cc: Jakub Kicinski Remove redefinition of GENMASK which should not be there in the upstream version of the code. Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> --- drivers/net/wireless/mediatek/mt7601u/regs.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/net/wireless/mediatek/mt7601u/regs.h b/drivers/net/wireless/mediatek/mt7601u/regs.h index afd8978e83fa..27a429d90cec 100644 --- a/drivers/net/wireless/mediatek/mt7601u/regs.h +++ b/drivers/net/wireless/mediatek/mt7601u/regs.h @@ -17,10 +17,6 @@ #include <linux/bitops.h> -#ifndef GENMASK -#define GENMASK(h, l) (((U32_C(1) << ((h) - (l) + 1)) - 1) << (l)) -#endif - #define MT_ASIC_VERSION 0x0000 #define MT76XX_REV_E3 0x22 -- 1.9.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv8 3/4] mt7601u: remove unnecessary include 2016-08-31 11:46 ` [PATCHv8 0/4] register-field manipulation macros Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 1/4] add basic " Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 2/4] mt7601u: remove redefinition of GENMASK Jakub Kicinski @ 2016-08-31 11:46 ` Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 4/4] mt7601u: use linux/bitfield.h Jakub Kicinski 2016-09-03 11:05 ` [PATCHv8 0/4] register-field manipulation macros Kalle Valo 4 siblings, 0 replies; 16+ messages in thread From: Jakub Kicinski @ 2016-08-31 11:46 UTC (permalink / raw) To: Kalle Valo, Linux Wireless List; +Cc: Jakub Kicinski There is no need to include linux/version.h in a in-tree driver. Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> --- drivers/net/wireless/mediatek/mt7601u/main.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/wireless/mediatek/mt7601u/main.c b/drivers/net/wireless/mediatek/mt7601u/main.c index e70dd9523911..43ebd460ba86 100644 --- a/drivers/net/wireless/mediatek/mt7601u/main.c +++ b/drivers/net/wireless/mediatek/mt7601u/main.c @@ -15,7 +15,6 @@ #include "mt7601u.h" #include "mac.h" #include <linux/etherdevice.h> -#include <linux/version.h> static int mt7601u_start(struct ieee80211_hw *hw) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv8 4/4] mt7601u: use linux/bitfield.h 2016-08-31 11:46 ` [PATCHv8 0/4] register-field manipulation macros Jakub Kicinski ` (2 preceding siblings ...) 2016-08-31 11:46 ` [PATCHv8 3/4] mt7601u: remove unnecessary include Jakub Kicinski @ 2016-08-31 11:46 ` Jakub Kicinski 2016-09-03 11:05 ` [PATCHv8 0/4] register-field manipulation macros Kalle Valo 4 siblings, 0 replies; 16+ messages in thread From: Jakub Kicinski @ 2016-08-31 11:46 UTC (permalink / raw) To: Kalle Valo, Linux Wireless List; +Cc: Jakub Kicinski Use the newly added linux/bitfield.h. Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com> Reviewed-by: Dinan Gunawardena <dinan.gunawardena@netronome.com> --- drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +- drivers/net/wireless/mediatek/mt7601u/dma.h | 10 ++-- drivers/net/wireless/mediatek/mt7601u/eeprom.c | 12 ++-- drivers/net/wireless/mediatek/mt7601u/init.c | 10 ++-- drivers/net/wireless/mediatek/mt7601u/mac.c | 38 ++++++------ drivers/net/wireless/mediatek/mt7601u/mcu.c | 20 +++---- drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +- drivers/net/wireless/mediatek/mt7601u/phy.c | 44 +++++++------- drivers/net/wireless/mediatek/mt7601u/tx.c | 19 +++--- drivers/net/wireless/mediatek/mt7601u/util.h | 77 ------------------------- 10 files changed, 81 insertions(+), 155 deletions(-) delete mode 100644 drivers/net/wireless/mediatek/mt7601u/util.h diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c index 57a80cfa39b1..a8bc064bc14f 100644 --- a/drivers/net/wireless/mediatek/mt7601u/dma.c +++ b/drivers/net/wireless/mediatek/mt7601u/dma.c @@ -103,7 +103,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev, u8 *data, if (unlikely(rxwi->zero[0] || rxwi->zero[1] || rxwi->zero[2])) dev_err_once(dev->dev, "Error: RXWI zero fields are set\n"); - if (unlikely(MT76_GET(MT_RXD_INFO_TYPE, fce_info))) + if (unlikely(FIELD_GET(MT_RXD_INFO_TYPE, fce_info))) dev_err_once(dev->dev, "Error: RX path seen a non-pkt urb\n"); trace_mt_rx(dev, rxwi, fce_info); diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.h b/drivers/net/wireless/mediatek/mt7601u/dma.h index 978e8a90b87f..270d126880c0 100644 --- a/drivers/net/wireless/mediatek/mt7601u/dma.h +++ b/drivers/net/wireless/mediatek/mt7601u/dma.h @@ -18,8 +18,6 @@ #include <asm/unaligned.h> #include <linux/skbuff.h> -#include "util.h" - #define MT_DMA_HDR_LEN 4 #define MT_RX_INFO_LEN 4 #define MT_FCE_INFO_LEN 4 @@ -79,9 +77,9 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb, */ info = flags | - MT76_SET(MT_TXD_INFO_LEN, round_up(skb->len, 4)) | - MT76_SET(MT_TXD_INFO_D_PORT, d_port) | - MT76_SET(MT_TXD_INFO_TYPE, type); + FIELD_PREP(MT_TXD_INFO_LEN, round_up(skb->len, 4)) | + FIELD_PREP(MT_TXD_INFO_D_PORT, d_port) | + FIELD_PREP(MT_TXD_INFO_TYPE, type); put_unaligned_le32(info, skb_push(skb, sizeof(info))); return skb_put_padto(skb, round_up(skb->len, 4) + 4); @@ -90,7 +88,7 @@ static inline int mt7601u_dma_skb_wrap(struct sk_buff *skb, static inline int mt7601u_dma_skb_wrap_pkt(struct sk_buff *skb, enum mt76_qsel qsel, u32 flags) { - flags |= MT76_SET(MT_TXD_PKT_INFO_QSEL, qsel); + flags |= FIELD_PREP(MT_TXD_PKT_INFO_QSEL, qsel); return mt7601u_dma_skb_wrap(skb, WLAN_PORT, DMA_PACKET, flags); } diff --git a/drivers/net/wireless/mediatek/mt7601u/eeprom.c b/drivers/net/wireless/mediatek/mt7601u/eeprom.c index 8d8ee0344f7b..da6faea092d6 100644 --- a/drivers/net/wireless/mediatek/mt7601u/eeprom.c +++ b/drivers/net/wireless/mediatek/mt7601u/eeprom.c @@ -45,8 +45,8 @@ mt7601u_efuse_read(struct mt7601u_dev *dev, u16 addr, u8 *data, val = mt76_rr(dev, MT_EFUSE_CTRL); val &= ~(MT_EFUSE_CTRL_AIN | MT_EFUSE_CTRL_MODE); - val |= MT76_SET(MT_EFUSE_CTRL_AIN, addr & ~0xf) | - MT76_SET(MT_EFUSE_CTRL_MODE, mode) | + val |= FIELD_PREP(MT_EFUSE_CTRL_AIN, addr & ~0xf) | + FIELD_PREP(MT_EFUSE_CTRL_MODE, mode) | MT_EFUSE_CTRL_KICK; mt76_wr(dev, MT_EFUSE_CTRL, val); @@ -128,8 +128,8 @@ mt7601u_set_chip_cap(struct mt7601u_dev *dev, u8 *eeprom) if (!field_valid(nic_conf0 >> 8)) return; - if (MT76_GET(MT_EE_NIC_CONF_0_RX_PATH, nic_conf0) > 1 || - MT76_GET(MT_EE_NIC_CONF_0_TX_PATH, nic_conf0) > 1) + if (FIELD_GET(MT_EE_NIC_CONF_0_RX_PATH, nic_conf0) > 1 || + FIELD_GET(MT_EE_NIC_CONF_0_TX_PATH, nic_conf0) > 1) dev_err(dev->dev, "Error: device has more than 1 RX/TX stream!\n"); } @@ -150,7 +150,7 @@ mt7601u_set_macaddr(struct mt7601u_dev *dev, const u8 *eeprom) mt76_wr(dev, MT_MAC_ADDR_DW0, get_unaligned_le32(dev->macaddr)); mt76_wr(dev, MT_MAC_ADDR_DW1, get_unaligned_le16(dev->macaddr + 4) | - MT76_SET(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff)); + FIELD_PREP(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff)); return 0; } @@ -176,7 +176,7 @@ mt7601u_set_channel_power(struct mt7601u_dev *dev, u8 *eeprom) u8 max_pwr; val = mt7601u_rr(dev, MT_TX_ALC_CFG_0); - max_pwr = MT76_GET(MT_TX_ALC_CFG_0_LIMIT_0, val); + max_pwr = FIELD_GET(MT_TX_ALC_CFG_0_LIMIT_0, val); if (mt7601u_has_tssi(dev, eeprom)) { mt7601u_set_channel_target_power(dev, eeprom, max_pwr); diff --git a/drivers/net/wireless/mediatek/mt7601u/init.c b/drivers/net/wireless/mediatek/mt7601u/init.c index 8fa78d7156be..44d46e25db80 100644 --- a/drivers/net/wireless/mediatek/mt7601u/init.c +++ b/drivers/net/wireless/mediatek/mt7601u/init.c @@ -108,8 +108,9 @@ static void mt7601u_init_usb_dma(struct mt7601u_dev *dev) { u32 val; - val = MT76_SET(MT_USB_DMA_CFG_RX_BULK_AGG_TOUT, MT_USB_AGGR_TIMEOUT) | - MT76_SET(MT_USB_DMA_CFG_RX_BULK_AGG_LMT, MT_USB_AGGR_SIZE_LIMIT) | + val = FIELD_PREP(MT_USB_DMA_CFG_RX_BULK_AGG_TOUT, MT_USB_AGGR_TIMEOUT) | + FIELD_PREP(MT_USB_DMA_CFG_RX_BULK_AGG_LMT, + MT_USB_AGGR_SIZE_LIMIT) | MT_USB_DMA_CFG_RX_BULK_EN | MT_USB_DMA_CFG_TX_BULK_EN; if (dev->in_max_packet == 512) @@ -396,8 +397,9 @@ int mt7601u_init_hardware(struct mt7601u_dev *dev) mt7601u_rmw(dev, MT_US_CYC_CFG, MT_US_CYC_CNT, 0x1e); - mt7601u_wr(dev, MT_TXOP_CTRL_CFG, MT76_SET(MT_TXOP_TRUN_EN, 0x3f) | - MT76_SET(MT_TXOP_EXT_CCA_DLY, 0x58)); + mt7601u_wr(dev, MT_TXOP_CTRL_CFG, + FIELD_PREP(MT_TXOP_TRUN_EN, 0x3f) | + FIELD_PREP(MT_TXOP_EXT_CCA_DLY, 0x58)); ret = mt7601u_eeprom_init(dev); if (ret) diff --git a/drivers/net/wireless/mediatek/mt7601u/mac.c b/drivers/net/wireless/mediatek/mt7601u/mac.c index e21c53ed09fb..3c576392ed89 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mac.c +++ b/drivers/net/wireless/mediatek/mt7601u/mac.c @@ -19,13 +19,13 @@ static void mt76_mac_process_tx_rate(struct ieee80211_tx_rate *txrate, u16 rate) { - u8 idx = MT76_GET(MT_TXWI_RATE_MCS, rate); + u8 idx = FIELD_GET(MT_TXWI_RATE_MCS, rate); txrate->idx = 0; txrate->flags = 0; txrate->count = 1; - switch (MT76_GET(MT_TXWI_RATE_PHY_MODE, rate)) { + switch (FIELD_GET(MT_TXWI_RATE_PHY_MODE, rate)) { case MT_PHY_TYPE_OFDM: txrate->idx = idx + 4; return; @@ -47,7 +47,7 @@ mt76_mac_process_tx_rate(struct ieee80211_tx_rate *txrate, u16 rate) return; } - if (MT76_GET(MT_TXWI_RATE_BW, rate) == MT_PHY_BW_40) + if (FIELD_GET(MT_TXWI_RATE_BW, rate) == MT_PHY_BW_40) txrate->flags |= IEEE80211_TX_RC_40_MHZ_WIDTH; if (rate & MT_TXWI_RATE_SGI) @@ -125,9 +125,9 @@ u16 mt76_mac_tx_rate_val(struct mt7601u_dev *dev, bw = 0; } - rateval = MT76_SET(MT_RXWI_RATE_MCS, rate_idx); - rateval |= MT76_SET(MT_RXWI_RATE_PHY, phy); - rateval |= MT76_SET(MT_RXWI_RATE_BW, bw); + rateval = FIELD_PREP(MT_RXWI_RATE_MCS, rate_idx); + rateval |= FIELD_PREP(MT_RXWI_RATE_PHY, phy); + rateval |= FIELD_PREP(MT_RXWI_RATE_BW, bw); if (rate->flags & IEEE80211_TX_RC_SHORT_GI) rateval |= MT_RXWI_RATE_SGI; @@ -156,9 +156,9 @@ struct mt76_tx_status mt7601u_mac_fetch_tx_status(struct mt7601u_dev *dev) stat.success = !!(val & MT_TX_STAT_FIFO_SUCCESS); stat.aggr = !!(val & MT_TX_STAT_FIFO_AGGR); stat.ack_req = !!(val & MT_TX_STAT_FIFO_ACKREQ); - stat.pktid = MT76_GET(MT_TX_STAT_FIFO_PID_TYPE, val); - stat.wcid = MT76_GET(MT_TX_STAT_FIFO_WCID, val); - stat.rate = MT76_GET(MT_TX_STAT_FIFO_RATE, val); + stat.pktid = FIELD_GET(MT_TX_STAT_FIFO_PID_TYPE, val); + stat.wcid = FIELD_GET(MT_TX_STAT_FIFO_WCID, val); + stat.rate = FIELD_GET(MT_TX_STAT_FIFO_RATE, val); return stat; } @@ -270,7 +270,7 @@ void mt7601u_mac_config_tsf(struct mt7601u_dev *dev, bool enable, int interval) } val &= ~MT_BEACON_TIME_CFG_INTVAL; - val |= MT76_SET(MT_BEACON_TIME_CFG_INTVAL, interval << 4) | + val |= FIELD_PREP(MT_BEACON_TIME_CFG_INTVAL, interval << 4) | MT_BEACON_TIME_CFG_TIMER_EN | MT_BEACON_TIME_CFG_SYNC_MODE | MT_BEACON_TIME_CFG_TBTT_EN; @@ -349,8 +349,8 @@ mt7601u_mac_wcid_setup(struct mt7601u_dev *dev, u8 idx, u8 vif_idx, u8 *mac) u8 zmac[ETH_ALEN] = {}; u32 attr; - attr = MT76_SET(MT_WCID_ATTR_BSS_IDX, vif_idx & 7) | - MT76_SET(MT_WCID_ATTR_BSS_IDX_EXT, !!(vif_idx & 8)); + attr = FIELD_PREP(MT_WCID_ATTR_BSS_IDX, vif_idx & 7) | + FIELD_PREP(MT_WCID_ATTR_BSS_IDX_EXT, !!(vif_idx & 8)); mt76_wr(dev, MT_WCID_ATTR(idx), attr); @@ -382,15 +382,15 @@ void mt7601u_mac_set_ampdu_factor(struct mt7601u_dev *dev) rcu_read_unlock(); mt7601u_wr(dev, MT_MAX_LEN_CFG, 0xa0fff | - MT76_SET(MT_MAX_LEN_CFG_AMPDU, min_factor)); + FIELD_PREP(MT_MAX_LEN_CFG_AMPDU, min_factor)); } static void mt76_mac_process_rate(struct ieee80211_rx_status *status, u16 rate) { - u8 idx = MT76_GET(MT_RXWI_RATE_MCS, rate); + u8 idx = FIELD_GET(MT_RXWI_RATE_MCS, rate); - switch (MT76_GET(MT_RXWI_RATE_PHY, rate)) { + switch (FIELD_GET(MT_RXWI_RATE_PHY, rate)) { case MT_PHY_TYPE_OFDM: if (WARN_ON(idx >= 8)) idx = 0; @@ -436,7 +436,7 @@ mt7601u_rx_monitor_beacon(struct mt7601u_dev *dev, struct mt7601u_rxwi *rxwi, u16 rate, int rssi) { dev->bcn_freq_off = rxwi->freq_off; - dev->bcn_phy_mode = MT76_GET(MT_RXWI_RATE_PHY, rate); + dev->bcn_phy_mode = FIELD_GET(MT_RXWI_RATE_PHY, rate); dev->avg_rssi = (dev->avg_rssi * 15) / 16 + (rssi << 8); } @@ -458,7 +458,7 @@ u32 mt76_mac_process_rx(struct mt7601u_dev *dev, struct sk_buff *skb, u16 rate = le16_to_cpu(rxwi->rate); int rssi; - len = MT76_GET(MT_RXWI_CTL_MPDU_LEN, ctl); + len = FIELD_GET(MT_RXWI_CTL_MPDU_LEN, ctl); if (len < 10) return 0; @@ -542,8 +542,8 @@ int mt76_mac_wcid_set_key(struct mt7601u_dev *dev, u8 idx, val = mt7601u_rr(dev, MT_WCID_ATTR(idx)); val &= ~MT_WCID_ATTR_PKEY_MODE & ~MT_WCID_ATTR_PKEY_MODE_EXT; - val |= MT76_SET(MT_WCID_ATTR_PKEY_MODE, cipher & 7) | - MT76_SET(MT_WCID_ATTR_PKEY_MODE_EXT, cipher >> 3); + val |= FIELD_PREP(MT_WCID_ATTR_PKEY_MODE, cipher & 7) | + FIELD_PREP(MT_WCID_ATTR_PKEY_MODE_EXT, cipher >> 3); val &= ~MT_WCID_ATTR_PAIRWISE; val |= MT_WCID_ATTR_PAIRWISE * !!(key && key->flags & IEEE80211_KEY_FLAG_PAIRWISE); diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c b/drivers/net/wireless/mediatek/mt7601u/mcu.c index 91c4b3427965..dbdfb3f5c507 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mcu.c +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c @@ -43,8 +43,8 @@ static inline void mt7601u_dma_skb_wrap_cmd(struct sk_buff *skb, u8 seq, enum mcu_cmd cmd) { WARN_ON(mt7601u_dma_skb_wrap(skb, CPU_TX_PORT, DMA_COMMAND, - MT76_SET(MT_TXD_CMD_INFO_SEQ, seq) | - MT76_SET(MT_TXD_CMD_INFO_TYPE, cmd))); + FIELD_PREP(MT_TXD_CMD_INFO_SEQ, seq) | + FIELD_PREP(MT_TXD_CMD_INFO_TYPE, cmd))); } static inline void trace_mt_mcu_msg_send_cs(struct mt7601u_dev *dev, @@ -100,13 +100,13 @@ static int mt7601u_mcu_wait_resp(struct mt7601u_dev *dev, u8 seq) dev_err(dev->dev, "Error: MCU resp urb failed:%d\n", urb_status); - if (MT76_GET(MT_RXD_CMD_INFO_CMD_SEQ, rxfce) == seq && - MT76_GET(MT_RXD_CMD_INFO_EVT_TYPE, rxfce) == CMD_DONE) + if (FIELD_GET(MT_RXD_CMD_INFO_CMD_SEQ, rxfce) == seq && + FIELD_GET(MT_RXD_CMD_INFO_EVT_TYPE, rxfce) == CMD_DONE) return 0; - dev_err(dev->dev, "Error: MCU resp evt:%hhx seq:%hhx-%hhx!\n", - MT76_GET(MT_RXD_CMD_INFO_EVT_TYPE, rxfce), - seq, MT76_GET(MT_RXD_CMD_INFO_CMD_SEQ, rxfce)); + dev_err(dev->dev, "Error: MCU resp evt:%lx seq:%hhx-%lx!\n", + FIELD_GET(MT_RXD_CMD_INFO_EVT_TYPE, rxfce), + seq, FIELD_GET(MT_RXD_CMD_INFO_CMD_SEQ, rxfce)); } dev_err(dev->dev, "Error: %s timed out\n", __func__); @@ -291,9 +291,9 @@ static int __mt7601u_dma_fw(struct mt7601u_dev *dev, u32 val; int ret; - reg = cpu_to_le32(MT76_SET(MT_TXD_INFO_TYPE, DMA_PACKET) | - MT76_SET(MT_TXD_INFO_D_PORT, CPU_TX_PORT) | - MT76_SET(MT_TXD_INFO_LEN, len)); + reg = cpu_to_le32(FIELD_PREP(MT_TXD_INFO_TYPE, DMA_PACKET) | + FIELD_PREP(MT_TXD_INFO_D_PORT, CPU_TX_PORT) | + FIELD_PREP(MT_TXD_INFO_LEN, len)); memcpy(buf.buf, ®, sizeof(reg)); memcpy(buf.buf + sizeof(reg), data, len); memset(buf.buf + sizeof(reg) + len, 0, 8); diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h index 428bd2f10b7b..c7ec40475a5f 100644 --- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h +++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h @@ -15,6 +15,7 @@ #ifndef MT7601U_H #define MT7601U_H +#include <linux/bitfield.h> #include <linux/kernel.h> #include <linux/device.h> #include <linux/mutex.h> @@ -24,7 +25,6 @@ #include <linux/debugfs.h> #include "regs.h" -#include "util.h" #define MT_CALIBRATE_INTERVAL (4 * HZ) @@ -299,7 +299,7 @@ bool mt76_poll_msec(struct mt7601u_dev *dev, u32 offset, u32 mask, u32 val, /* Compatibility with mt76 */ #define mt76_rmw_field(_dev, _reg, _field, _val) \ - mt76_rmw(_dev, _reg, _field, MT76_SET(_field, _val)) + mt76_rmw(_dev, _reg, _field, FIELD_PREP(_field, _val)) static inline u32 mt76_rr(struct mt7601u_dev *dev, u32 offset) { diff --git a/drivers/net/wireless/mediatek/mt7601u/phy.c b/drivers/net/wireless/mediatek/mt7601u/phy.c index 1908af6add87..ca09a5d4305e 100644 --- a/drivers/net/wireless/mediatek/mt7601u/phy.c +++ b/drivers/net/wireless/mediatek/mt7601u/phy.c @@ -41,11 +41,12 @@ mt7601u_rf_wr(struct mt7601u_dev *dev, u8 bank, u8 offset, u8 value) goto out; } - mt7601u_wr(dev, MT_RF_CSR_CFG, MT76_SET(MT_RF_CSR_CFG_DATA, value) | - MT76_SET(MT_RF_CSR_CFG_REG_BANK, bank) | - MT76_SET(MT_RF_CSR_CFG_REG_ID, offset) | - MT_RF_CSR_CFG_WR | - MT_RF_CSR_CFG_KICK); + mt7601u_wr(dev, MT_RF_CSR_CFG, + FIELD_PREP(MT_RF_CSR_CFG_DATA, value) | + FIELD_PREP(MT_RF_CSR_CFG_REG_BANK, bank) | + FIELD_PREP(MT_RF_CSR_CFG_REG_ID, offset) | + MT_RF_CSR_CFG_WR | + MT_RF_CSR_CFG_KICK); trace_rf_write(dev, bank, offset, value); out: mutex_unlock(&dev->reg_atomic_mutex); @@ -74,17 +75,18 @@ mt7601u_rf_rr(struct mt7601u_dev *dev, u8 bank, u8 offset) if (!mt76_poll(dev, MT_RF_CSR_CFG, MT_RF_CSR_CFG_KICK, 0, 100)) goto out; - mt7601u_wr(dev, MT_RF_CSR_CFG, MT76_SET(MT_RF_CSR_CFG_REG_BANK, bank) | - MT76_SET(MT_RF_CSR_CFG_REG_ID, offset) | - MT_RF_CSR_CFG_KICK); + mt7601u_wr(dev, MT_RF_CSR_CFG, + FIELD_PREP(MT_RF_CSR_CFG_REG_BANK, bank) | + FIELD_PREP(MT_RF_CSR_CFG_REG_ID, offset) | + MT_RF_CSR_CFG_KICK); if (!mt76_poll(dev, MT_RF_CSR_CFG, MT_RF_CSR_CFG_KICK, 0, 100)) goto out; val = mt7601u_rr(dev, MT_RF_CSR_CFG); - if (MT76_GET(MT_RF_CSR_CFG_REG_ID, val) == offset && - MT76_GET(MT_RF_CSR_CFG_REG_BANK, val) == bank) { - ret = MT76_GET(MT_RF_CSR_CFG_DATA, val); + if (FIELD_GET(MT_RF_CSR_CFG_REG_ID, val) == offset && + FIELD_GET(MT_RF_CSR_CFG_REG_BANK, val) == bank) { + ret = FIELD_GET(MT_RF_CSR_CFG_DATA, val); trace_rf_read(dev, bank, offset, ret); } out: @@ -139,8 +141,8 @@ static void mt7601u_bbp_wr(struct mt7601u_dev *dev, u8 offset, u8 val) } mt7601u_wr(dev, MT_BBP_CSR_CFG, - MT76_SET(MT_BBP_CSR_CFG_VAL, val) | - MT76_SET(MT_BBP_CSR_CFG_REG_NUM, offset) | + FIELD_PREP(MT_BBP_CSR_CFG_VAL, val) | + FIELD_PREP(MT_BBP_CSR_CFG_REG_NUM, offset) | MT_BBP_CSR_CFG_RW_MODE | MT_BBP_CSR_CFG_BUSY); trace_bbp_write(dev, offset, val); out: @@ -163,7 +165,7 @@ static int mt7601u_bbp_rr(struct mt7601u_dev *dev, u8 offset) goto out; mt7601u_wr(dev, MT_BBP_CSR_CFG, - MT76_SET(MT_BBP_CSR_CFG_REG_NUM, offset) | + FIELD_PREP(MT_BBP_CSR_CFG_REG_NUM, offset) | MT_BBP_CSR_CFG_RW_MODE | MT_BBP_CSR_CFG_BUSY | MT_BBP_CSR_CFG_READ); @@ -171,8 +173,8 @@ static int mt7601u_bbp_rr(struct mt7601u_dev *dev, u8 offset) goto out; val = mt7601u_rr(dev, MT_BBP_CSR_CFG); - if (MT76_GET(MT_BBP_CSR_CFG_REG_NUM, val) == offset) { - ret = MT76_GET(MT_BBP_CSR_CFG_VAL, val); + if (FIELD_GET(MT_BBP_CSR_CFG_REG_NUM, val) == offset) { + ret = FIELD_GET(MT_BBP_CSR_CFG_VAL, val); trace_bbp_read(dev, offset, ret); } out: @@ -249,9 +251,9 @@ int mt7601u_phy_get_rssi(struct mt7601u_dev *dev, /* bw40 */ { -2, 16, 34 } } }; - int bw = MT76_GET(MT_RXWI_RATE_BW, rate); - int aux_lna = MT76_GET(MT_RXWI_ANT_AUX_LNA, rxwi->ant); - int lna_id = MT76_GET(MT_RXWI_GAIN_RSSI_LNA_ID, rxwi->gain); + int bw = FIELD_GET(MT_RXWI_RATE_BW, rate); + int aux_lna = FIELD_GET(MT_RXWI_ANT_AUX_LNA, rxwi->ant); + int lna_id = FIELD_GET(MT_RXWI_GAIN_RSSI_LNA_ID, rxwi->gain); int val; if (lna_id) /* LNA id can be 0, 2, 3. */ @@ -259,7 +261,7 @@ int mt7601u_phy_get_rssi(struct mt7601u_dev *dev, val = 8; val -= lna[aux_lna][bw][lna_id]; - val -= MT76_GET(MT_RXWI_GAIN_RSSI_VAL, rxwi->gain); + val -= FIELD_GET(MT_RXWI_GAIN_RSSI_VAL, rxwi->gain); val -= dev->ee->lna_gain; val -= dev->ee->rssi_offset[0]; @@ -939,7 +941,7 @@ static int mt7601u_tssi_cal(struct mt7601u_dev *dev) dev_dbg(dev->dev, "final diff: %08x\n", diff_pwr); val = mt7601u_rr(dev, MT_TX_ALC_CFG_1); - curr_pwr = s6_to_int(MT76_GET(MT_TX_ALC_CFG_1_TEMP_COMP, val)); + curr_pwr = s6_to_int(FIELD_GET(MT_TX_ALC_CFG_1_TEMP_COMP, val)); diff_pwr += curr_pwr; val = (val & ~MT_TX_ALC_CFG_1_TEMP_COMP) | int_to_s6(diff_pwr); mt7601u_wr(dev, MT_TX_ALC_CFG_1, val); diff --git a/drivers/net/wireless/mediatek/mt7601u/tx.c b/drivers/net/wireless/mediatek/mt7601u/tx.c index a0a33dc8f6bc..ad77bec1ba0f 100644 --- a/drivers/net/wireless/mediatek/mt7601u/tx.c +++ b/drivers/net/wireless/mediatek/mt7601u/tx.c @@ -175,11 +175,12 @@ mt7601u_push_txwi(struct mt7601u_dev *dev, struct sk_buff *skb, ba_size = min_t(int, 63, ba_size); if (info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE) ba_size = 0; - txwi->ack_ctl |= MT76_SET(MT_TXWI_ACK_CTL_BA_WINDOW, ba_size); + txwi->ack_ctl |= FIELD_PREP(MT_TXWI_ACK_CTL_BA_WINDOW, ba_size); - txwi->flags = cpu_to_le16(MT_TXWI_FLAGS_AMPDU | - MT76_SET(MT_TXWI_FLAGS_MPDU_DENSITY, - sta->ht_cap.ampdu_density)); + txwi->flags = + cpu_to_le16(MT_TXWI_FLAGS_AMPDU | + FIELD_PREP(MT_TXWI_FLAGS_MPDU_DENSITY, + sta->ht_cap.ampdu_density)); if (info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE) txwi->flags = 0; } @@ -188,7 +189,7 @@ mt7601u_push_txwi(struct mt7601u_dev *dev, struct sk_buff *skb, is_probe = !!(info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE); pkt_id = mt7601u_tx_pktid_enc(dev, rate_ctl & 0x7, is_probe); - pkt_len |= MT76_SET(MT_TXWI_LEN_PKTID, pkt_id); + pkt_len |= FIELD_PREP(MT_TXWI_LEN_PKTID, pkt_id); txwi->len_ctl = cpu_to_le16(pkt_len); return txwi; @@ -285,9 +286,9 @@ int mt7601u_conf_tx(struct ieee80211_hw *hw, struct ieee80211_vif *vif, WARN_ON(cw_min > 0xf); WARN_ON(cw_max > 0xf); - val = MT76_SET(MT_EDCA_CFG_AIFSN, params->aifs) | - MT76_SET(MT_EDCA_CFG_CWMIN, cw_min) | - MT76_SET(MT_EDCA_CFG_CWMAX, cw_max); + val = FIELD_PREP(MT_EDCA_CFG_AIFSN, params->aifs) | + FIELD_PREP(MT_EDCA_CFG_CWMIN, cw_min) | + FIELD_PREP(MT_EDCA_CFG_CWMAX, cw_max); /* TODO: based on user-controlled EnableTxBurst var vendor drv sets * a really long txop on AC0 (see connect.c:2009) but only on * connect? When not connected should be 0. @@ -295,7 +296,7 @@ int mt7601u_conf_tx(struct ieee80211_hw *hw, struct ieee80211_vif *vif, if (!hw_q) val |= 0x60; else - val |= MT76_SET(MT_EDCA_CFG_TXOP, params->txop); + val |= FIELD_PREP(MT_EDCA_CFG_TXOP, params->txop); mt76_wr(dev, MT_EDCA_CFG_AC(hw_q), val); val = mt76_rr(dev, MT_WMM_TXOP(hw_q)); diff --git a/drivers/net/wireless/mediatek/mt7601u/util.h b/drivers/net/wireless/mediatek/mt7601u/util.h deleted file mode 100644 index b89140bf1210..000000000000 --- a/drivers/net/wireless/mediatek/mt7601u/util.h +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright (C) 2014 Felix Fietkau <nbd@openwrt.org> - * Copyright (C) 2004 - 2009 Ivo van Doorn <IvDoorn@gmail.com> - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#ifndef __MT76_UTIL_H -#define __MT76_UTIL_H - -/* - * Power of two check, this will check - * if the mask that has been given contains and contiguous set of bits. - * Note that we cannot use the is_power_of_2() function since this - * check must be done at compile-time. - */ -#define is_power_of_two(x) ( !((x) & ((x)-1)) ) -#define low_bit_mask(x) ( ((x)-1) & ~(x) ) -#define is_valid_mask(x) is_power_of_two(1LU + (x) + low_bit_mask(x)) - -/* - * Macros to find first set bit in a variable. - * These macros behave the same as the __ffs() functions but - * the most important difference that this is done during - * compile-time rather then run-time. - */ -#define compile_ffs2(__x) \ - __builtin_choose_expr(((__x) & 0x1), 0, 1) - -#define compile_ffs4(__x) \ - __builtin_choose_expr(((__x) & 0x3), \ - (compile_ffs2((__x))), \ - (compile_ffs2((__x) >> 2) + 2)) - -#define compile_ffs8(__x) \ - __builtin_choose_expr(((__x) & 0xf), \ - (compile_ffs4((__x))), \ - (compile_ffs4((__x) >> 4) + 4)) - -#define compile_ffs16(__x) \ - __builtin_choose_expr(((__x) & 0xff), \ - (compile_ffs8((__x))), \ - (compile_ffs8((__x) >> 8) + 8)) - -#define compile_ffs32(__x) \ - __builtin_choose_expr(((__x) & 0xffff), \ - (compile_ffs16((__x))), \ - (compile_ffs16((__x) >> 16) + 16)) - -/* - * This macro will check the requirements for the FIELD{8,16,32} macros - * The mask should be a constant non-zero contiguous set of bits which - * does not exceed the given typelimit. - */ -#define FIELD_CHECK(__mask) \ - BUILD_BUG_ON(!(__mask) || !is_valid_mask(__mask)) - -#define MT76_SET(_mask, _val) \ - ({ \ - FIELD_CHECK(_mask); \ - (((u32) (_val)) << compile_ffs32(_mask)) & _mask; \ - }) - -#define MT76_GET(_mask, _val) \ - ({ \ - FIELD_CHECK(_mask); \ - (u32) (((_val) & _mask) >> compile_ffs32(_mask)); \ - }) - -#endif -- 1.9.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCHv8 0/4] register-field manipulation macros 2016-08-31 11:46 ` [PATCHv8 0/4] register-field manipulation macros Jakub Kicinski ` (3 preceding siblings ...) 2016-08-31 11:46 ` [PATCHv8 4/4] mt7601u: use linux/bitfield.h Jakub Kicinski @ 2016-09-03 11:05 ` Kalle Valo 4 siblings, 0 replies; 16+ messages in thread From: Kalle Valo @ 2016-09-03 11:05 UTC (permalink / raw) To: Jakub Kicinski; +Cc: Linux Wireless List, linux-kernel Jakub Kicinski <jakub.kicinski@netronome.com> writes: > Small improvement suggested by Daniel Borkmann - use > two underscore prefix. > > https://www.mail-archive.com/netdev@vger.kernel.org/msg125423.html > > -- v6 blurb: > > This set moves to a global header file macros which I find > very useful and worth popularising. The basic problem is > that since C bitfields are not very dependable accessing > subfields of registers becomes slightly inconvenient. > It is nice to have the necessary mask and shift operations > wrapped in a macro and have that macro compute shift > at compilation time using FFS. > > My implementation follows what Felix Fietkau has done in > mt76. Hannes Frederic Sowa suggested more use of standard > Linux/GCC functions. Since the RFC I've also added a > compile-time check to validate that the value passed to > setters fits in the mask. > > I attempted the use of static inlines instead of macros > but it makes GCC < 6.0 barf at the BUILD_BUG_ON()s. > I also noticed that forcing arguments to be u32 for inlines > makes the compiler use 32bit arithmetic where it could > get away with 64bit before (on 64bit machines, obviously). > That's a potential performance concern but probably not > a very practical one today. Apart from looking "cleaner" > static inlines would have the advantage that we could #undef > the auxiliary macros at the end of the header. > > Build bot caught a build failure with -Os set. AFAICT gcc > did not handle temporary variable I put in the macro > expression too well. I work around that by defining > __BUILD_BUG_ON_NOT_POWER_OF_2 and using it instead of > BUILD_BUG_ON(!tmp || is_power_of_2(tmp)). > > I'm planning to use those macros in another driver soon, > Felix may also use them when upstreaming mt76 if he chooses > to do so. > > IMHO if accepted it would be best to push these through > Kalle's wireless drivers' tree, since the only existing > user is in drivers/net/wireless/. > > v8: > - use two underscores for auxiliary macros (Daniel B). > v7: > - drop the explicit type marking (u32/u64) - depend on the type > of the mask instead; > - only allow compilation time constant masks; > - barf at "type of register too small to ever match mask" on get; > - rename PUT -> PREP. > v6: > - do a full rename in patch 2; > - CC many people. > v5: > - repost. > v4: > - add documentation in the header. > v3: > - don't use variables in statement expressions; > - use __BUILD_BUG_ON_NOT_POWER_OF_2. > v2: > - change Felix's email address. > > Jakub Kicinski (4): > add basic register-field manipulation macros > mt7601u: remove redefinition of GENMASK > mt7601u: remove unnecessary include > mt7601u: use linux/bitfield.h I applied these now to the pending branch of my wireless-drivers-next tree. Let's see if the kbuild bot finds any problems. Please also CC lkml, did that now. -- Kalle Valo ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-09-09 9:11 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-08-19 16:44 [PATCHv7 0/4] register-field manipulation macros Jakub Kicinski 2016-08-19 16:44 ` [PATCHv7 1/4] add basic " Jakub Kicinski 2016-08-19 16:44 ` [PATCHv7 2/4] mt7601u: remove redefinition of GENMASK Jakub Kicinski 2016-08-19 16:44 ` [PATCHv7 3/4] mt7601u: remove unnecessary include Jakub Kicinski 2016-08-19 16:44 ` [PATCHv7 4/4] mt7601u: use linux/bitfield.h Jakub Kicinski 2016-08-19 19:02 ` Arend Van Spriel 2016-08-22 10:52 ` Jakub Kicinski 2016-08-22 19:19 ` Arend Van Spriel 2016-08-22 20:59 ` Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 0/4] register-field manipulation macros Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 1/4] add basic " Jakub Kicinski 2016-09-09 9:11 ` [PATCHv8,1/4] " Kalle Valo 2016-08-31 11:46 ` [PATCHv8 2/4] mt7601u: remove redefinition of GENMASK Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 3/4] mt7601u: remove unnecessary include Jakub Kicinski 2016-08-31 11:46 ` [PATCHv8 4/4] mt7601u: use linux/bitfield.h Jakub Kicinski 2016-09-03 11:05 ` [PATCHv8 0/4] register-field manipulation macros Kalle Valo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).