bitops: Provide generic sign_extend function (moving it out from wireless code)
Subsystems (4)
PATCH 1/1 bitops: Provide generic sign_extend function (moving it out from wireless code)
From: Andreas Herrmann <[email protected]>
Provide a common function to sign extend a value.
Signed-off-by Andreas Herrmann <[email protected]>
--- drivers/net/wireless/ath/ath5k/phy.c | 6 ------ drivers/net/wireless/ath/ath9k/hw.h | 6 ------ drivers/net/wireless/iwlwifi/iwl-4965.c | 16 ---------------- include/linux/bitops.h | 11 +++++++++++ 4 files changed, 11 insertions(+), 28 deletions(-)
Recently I needed to sign extend some register values for further computation. I also stumbled over two functions in wireless code which do the same (in different ways).
Thus I wonder whether a generic function should be provided for this.
Below patch (against tip/master) provides the iwl-4965-variant of that function in bitops.h I am not sure whether that's the right place to add this function. What do you think? Or is it a dumb idea anyway?
Thanks,
Andreas
drivers/net/wireless/ath/ath5k/phy.c +0 -6
@@ -1101,12 +1101,6 @@ int ath5k_hw_channel(struct ath5k_hw *ah, struct ieee80211_channel *channel)PHY calibration \*****************/ -static int sign_extend (int val,const int nbits) -{ -int order =BIT (nbits-1 ); -return (val ^ order) - order; -} -static s32 ath5k_hw_read_measured_noise_floor (struct ath5k_hw *ah) {s32 val;
drivers/net/wireless/ath/ath9k/hw.h +0 -6
@@ -858,12 +858,6 @@ static inline struct ath_hw_ops *ath9k_hw_ops(struct ath_hw *ah)return &ah->ops; } -static inline int sign_extend (int val,const int nbits) -{ -int order =BIT (nbits-1 ); -return (val ^ order) - order; -} -/* Initialization, Detach, Reset */ const char *ath9k_hw_probe (u16 vendorid,u16 devid);void ath9k_hw_deinit (struct ath_hw *ah);
drivers/net/wireless/iwlwifi/iwl-4965.c +0 -16
@@ -1551,22 +1551,6 @@ static void iwl4965_txq_update_byte_cnt_tbl(struct iwl_priv *priv, }/** -* sign_extend - Sign extend a value using specified bit as sign-bit -* -* Example: sign_extend(9, 3) would return -7 as bit3 of 1001b is 1 -* and bit0..2 is 001b which when sign extended to 1111111111111001b is -7. -* -* @param oper value to sign extend -* @param index 0 based bit index (0<=index<32) to sign bit -*/ -static s32 sign_extend (u32 oper,int index) -{ -u8 shift =31 - index; - -return (s32)(oper << shift) >> shift; -} - -/** * iwl4965_hw_get_temperature - return the calibrated temperature (in Kelvin) * @statistics: Provides the temperature reading from the uCode *
include/linux/bitops.h +11 -0
@@ -109,6 +109,17 @@ static inline __u8 ror8(__u8 word, unsigned int shift)return (word >> shift) | (word << (8 - shift)); } +/** +* sign_extend - Sign extend a value using specified bit as sign-bit +* @value: value to sign extend +* @index: 0 based bit index (0<=index<32) to sign bit +*/ +static inline __s32 sign_extend (__u32 value,int index) +{ +__u8 shift =31 - index; +return (__s32)(value << shift) >> shift; +} +static inline unsigned fls_long (unsigned long l) {if (sizeof (l) ==4 )
Signature · 1 line
1.6.4.4
include/linux/bitops.h static inline __u8 ror8(__u8 word, unsigned int shift) · line 121
Quoted 11 lines · Andreas Herrmann
include/linux/bitops.h +10
-0+/** +* sign_extend - Sign extend a value using specified bit as sign-bit +* @value: value to sign extend +* @index: 0 based bit index (0<=index<32) to sign bit +*/ +static inline __s32 sign_extend (__u32 value,int index) +{ +__u8 shift =31 - index; +return (__s32)(value << shift) >> shift; +}
If it only handles 32bit please call it sign_extend32
-Andi
Signature · 1 line
[email protected] -- Speaking for myself only.
Quoted 16 lines · Andreas Herrmann
From: Andreas Herrmann <[email protected]>
Provide a common function to sign extend a value.
Signed-off-by Andreas Herrmann <[email protected]>--- drivers/net/wireless/ath/ath5k/phy.c | 6 ------ drivers/net/wireless/ath/ath9k/hw.h | 6 ------ drivers/net/wireless/iwlwifi/iwl-4965.c | 16 ---------------- include/linux/bitops.h | 11 +++++++++++ 4 files changed, 11 insertions(+), 28 deletions(-)Recently I needed to sign extend some register values for further computation. I also stumbled over two functions in wireless code which do the same (in different ways).
Quoted 5 lines
Thus I wonder whether a generic function should be provided for this.
Below patch (against tip/master) provides the iwl-4965-variant of that function in
bitops.hI am not sure whether that's the right place to add this function. What do you think? Or is it a dumb idea anyway?
Think its reasonable. Adding linux-wireless.
Luis
Quoted 91 lines
Thanks,
Andreasdrivers/net/wireless/ath/ath5k/phy.c +0
-6@@ -1101,12 +1101,6 @@ int ath5k_hw_channel(struct ath5k_hw *ah, struct ieee80211_channel *channel)PHY calibration \*****************/ -static int sign_extend (int val,const int nbits) -{ -int order =BIT (nbits-1 ); -return (val ^ order) - order; -} -static s32 ath5k_hw_read_measured_noise_floor (struct ath5k_hw *ah) {s32 val;drivers/net/wireless/ath/ath9k/hw.h +0
-6@@ -858,12 +858,6 @@ static inline struct ath_hw_ops *ath9k_hw_ops(struct ath_hw *ah)return &ah->ops; } -static inline int sign_extend (int val,const int nbits) -{ -int order =BIT (nbits-1 ); -return (val ^ order) - order; -} -/* Initialization, Detach, Reset */ const char *ath9k_hw_probe (u16 vendorid,u16 devid);void ath9k_hw_deinit (struct ath_hw *ah);drivers/net/wireless/iwlwifi/iwl-4965.c +0
-16@@ -1551,22 +1551,6 @@ static void iwl4965_txq_update_byte_cnt_tbl(struct iwl_priv *priv, }/** -* sign_extend - Sign extend a value using specified bit as sign-bit -* -* Example: sign_extend(9, 3) would return -7 as bit3 of 1001b is 1 -* and bit0..2 is 001b which when sign extended to 1111111111111001b is -7. -* -* @param oper value to sign extend -* @param index 0 based bit index (0<=index<32) to sign bit -*/ -static s32 sign_extend (u32 oper,int index) -{ -u8 shift =31 - index; - -return (s32)(oper << shift) >> shift; -} - -/** * iwl4965_hw_get_temperature - return the calibrated temperature (in Kelvin) * @statistics: Provides the temperature reading from the uCode * include/linux/bitops.h +11
-0@@ -109,6 +109,17 @@ static inline __u8 ror8(__u8 word, unsigned int shift)return (word >> shift) | (word << (8 - shift)); } +/** +* sign_extend - Sign extend a value using specified bit as sign-bit +* @value: value to sign extend +* @index: 0 based bit index (0<=index<32) to sign bit +*/ +static inline __s32 sign_extend (__u32 value,int index) +{ +__u8 shift =31 - index; +return (__s32)(value << shift) >> shift; +} +static inline unsigned fls_long (unsigned long l) {if (sizeof (l) ==4 )Signature · 1 line
1.6.4.4
Review
Quoted 2 lines · Andreas Herrmann
(quoted diff) +0
-2-static int sign_extend(int val, const int nbits) -static s32 sign_extend(u32 oper, int index)
Note these two are specified slightly differently, so return different values with the same arguments. The ath[59]k version takes the number of bits (1-based) while the other one takes the position of the high order bit (0-based). I think the former is a little more natural but it's a matter of taste.
Anyway I personally have no problem with merging them once the above is fixed. I wrote a similar patch when I wrote the ath5k version, but never bothered to submit it.
Signature · 1 line
Bob Copeland %% www.bobcopeland.com
PATCH v2 1/1 bitops: Provide generic sign_extend32 function
In reply to Andreas HerrmannFrom: Andreas Herrmann <[email protected]>
This patch moves code out from wireless drivers where two different functions are defined in three code locations for the same purpose and provides a common function to sign extend a 32-bit value.
Signed-off-by Andreas Herrmann <[email protected]>
--- drivers/net/wireless/ath/ath5k/phy.c | 8 +------- drivers/net/wireless/ath/ath9k/ar5008_phy.c | 12 ++++++------ drivers/net/wireless/ath/ath9k/ar9002_phy.c | 8 ++++---- drivers/net/wireless/ath/ath9k/ar9003_phy.c | 12 ++++++------ drivers/net/wireless/ath/ath9k/hw.h | 6 ------ drivers/net/wireless/iwlwifi/iwl-4965.c | 20 ++------------------ include/linux/bitops.h | 11 +++++++++++ 7 files changed, 30 insertions(+), 47 deletions(-)
Differences to first version.
(1) Rename to sign_extend32 - it handles only 32-bit values
(2) Adapt function calls in ath*k code - the common function uses bit
position (0..31) instead of number of bits (1..32)
Andreas
drivers/net/wireless/ath/ath5k/phy.c +1 -7
@@ -1101,18 +1101,12 @@ int ath5k_hw_channel(struct ath5k_hw *ah, struct ieee80211_channel *channel)PHY calibration \*****************/ -static int sign_extend (int val,const int nbits) -{ -int order =BIT (nbits-1 ); -return (val ^ order) - order; -} -static s32 ath5k_hw_read_measured_noise_floor (struct ath5k_hw *ah) {s32 val; val =ath5k_hw_reg_read (ah,AR5K_PHY_NF ); -return sign_extend (AR5K_REG_MS (val,AR5K_PHY_NF_MINCCA_PWR ),9 ); +return sign_extend32 (AR5K_REG_MS (val,AR5K_PHY_NF_MINCCA_PWR ),8 ); }void ath5k_hw_init_nfcal_hist (struct ath5k_hw *ah)
drivers/net/wireless/ath/ath9k/ar5008_phy.c +6 -6
@@ -1498,25 +1498,25 @@ static void ar5008_hw_do_getnf(struct ath_hw *ah,int16_t nf; nf =MS (REG_READ (ah,AR_PHY_CCA ),AR_PHY_MINCCA_PWR ); - nfarray[0 ] =sign_extend (nf,9 ); + nfarray[0 ] =sign_extend32 (nf,8 ); nf =MS (REG_READ (ah,AR_PHY_CH1_CCA ),AR_PHY_CH1_MINCCA_PWR ); - nfarray[1 ] =sign_extend (nf,9 ); + nfarray[1 ] =sign_extend32 (nf,8 ); nf =MS (REG_READ (ah,AR_PHY_CH2_CCA ),AR_PHY_CH2_MINCCA_PWR ); - nfarray[2 ] =sign_extend (nf,9 ); + nfarray[2 ] =sign_extend32 (nf,8 );if (!IS_CHAN_HT40 (ah->curchan))return ; nf =MS (REG_READ (ah,AR_PHY_EXT_CCA ),AR_PHY_EXT_MINCCA_PWR ); - nfarray[3 ] =sign_extend (nf,9 ); + nfarray[3 ] =sign_extend32 (nf,8 ); nf =MS (REG_READ (ah,AR_PHY_CH1_EXT_CCA ),AR_PHY_CH1_EXT_MINCCA_PWR ); - nfarray[4 ] =sign_extend (nf,9 ); + nfarray[4 ] =sign_extend32 (nf,8 ); nf =MS (REG_READ (ah,AR_PHY_CH2_EXT_CCA ),AR_PHY_CH2_EXT_MINCCA_PWR ); - nfarray[5 ] =sign_extend (nf,9 ); + nfarray[5 ] =sign_extend32 (nf,8 ); }/*
drivers/net/wireless/ath/ath9k/ar9002_phy.c +4 -4
@@ -474,21 +474,21 @@ static void ar9002_hw_do_getnf(struct ath_hw *ah,int16_t nf; nf =MS (REG_READ (ah,AR_PHY_CCA ),AR9280_PHY_MINCCA_PWR ); - nfarray[0 ] =sign_extend (nf,9 ); + nfarray[0 ] =sign_extend32 (nf,8 ); nf =MS (REG_READ (ah,AR_PHY_EXT_CCA ),AR9280_PHY_EXT_MINCCA_PWR );if (IS_CHAN_HT40 (ah->curchan)) - nfarray[3 ] =sign_extend (nf,9 ); + nfarray[3 ] =sign_extend32 (nf,8 );if (AR_SREV_9285 (ah) ||AR_SREV_9271 (ah))return ; nf =MS (REG_READ (ah,AR_PHY_CH1_CCA ),AR9280_PHY_CH1_MINCCA_PWR ); - nfarray[1 ] =sign_extend (nf,9 ); + nfarray[1 ] =sign_extend32 (nf,8 ); nf =MS (REG_READ (ah,AR_PHY_CH1_EXT_CCA ),AR9280_PHY_CH1_EXT_MINCCA_PWR );if (IS_CHAN_HT40 (ah->curchan)) - nfarray[4 ] =sign_extend (nf,9 ); + nfarray[4 ] =sign_extend32 (nf,8 ); }static void ar9002_hw_set_nf_limits (struct ath_hw *ah)
drivers/net/wireless/ath/ath9k/ar9003_phy.c +6 -6
@@ -1025,25 +1025,25 @@ static void ar9003_hw_do_getnf(struct ath_hw *ah,int16_t nf; nf =MS (REG_READ (ah,AR_PHY_CCA_0 ),AR_PHY_MINCCA_PWR ); - nfarray[0 ] =sign_extend (nf,9 ); + nfarray[0 ] =sign_extend32 (nf,8 ); nf =MS (REG_READ (ah,AR_PHY_CCA_1 ),AR_PHY_CH1_MINCCA_PWR ); - nfarray[1 ] =sign_extend (nf,9 ); + nfarray[1 ] =sign_extend32 (nf,8 ); nf =MS (REG_READ (ah,AR_PHY_CCA_2 ),AR_PHY_CH2_MINCCA_PWR ); - nfarray[2 ] =sign_extend (nf,9 ); + nfarray[2 ] =sign_extend32 (nf,8 );if (!IS_CHAN_HT40 (ah->curchan))return ; nf =MS (REG_READ (ah,AR_PHY_EXT_CCA ),AR_PHY_EXT_MINCCA_PWR ); - nfarray[3 ] =sign_extend (nf,9 ); + nfarray[3 ] =sign_extend32 (nf,8 ); nf =MS (REG_READ (ah,AR_PHY_EXT_CCA_1 ),AR_PHY_CH1_EXT_MINCCA_PWR ); - nfarray[4 ] =sign_extend (nf,9 ); + nfarray[4 ] =sign_extend32 (nf,8 ); nf =MS (REG_READ (ah,AR_PHY_EXT_CCA_2 ),AR_PHY_CH2_EXT_MINCCA_PWR ); - nfarray[5 ] =sign_extend (nf,9 ); + nfarray[5 ] =sign_extend32 (nf,8 ); }static void ar9003_hw_set_nf_limits (struct ath_hw *ah)
drivers/net/wireless/ath/ath9k/hw.h +0 -6
@@ -858,12 +858,6 @@ static inline struct ath_hw_ops *ath9k_hw_ops(struct ath_hw *ah)return &ah->ops; } -static inline int sign_extend (int val,const int nbits) -{ -int order =BIT (nbits-1 ); -return (val ^ order) - order; -} -/* Initialization, Detach, Reset */ const char *ath9k_hw_probe (u16 vendorid,u16 devid);void ath9k_hw_deinit (struct ath_hw *ah);
drivers/net/wireless/iwlwifi/iwl-4965.c +2 -18
@@ -1551,22 +1551,6 @@ static void iwl4965_txq_update_byte_cnt_tbl(struct iwl_priv *priv, }/** -* sign_extend - Sign extend a value using specified bit as sign-bit -* -* Example: sign_extend(9, 3) would return -7 as bit3 of 1001b is 1 -* and bit0..2 is 001b which when sign extended to 1111111111111001b is -7. -* -* @param oper value to sign extend -* @param index 0 based bit index (0<=index<32) to sign bit -*/ -static s32 sign_extend (u32 oper,int index) -{ -u8 shift =31 - index; - -return (s32)(oper << shift) >> shift; -} - -/** * iwl4965_hw_get_temperature - return the calibrated temperature (in Kelvin) * @statistics: Provides the temperature reading from the uCode *
@@ -1603,9 +1587,9 @@ static int iwl4965_hw_get_temperature(struct iwl_priv *priv)* "initialize" ALIVE response. */ if (!test_bit (STATUS_TEMPERATURE , &priv->status)) - vt =sign_extend (R4 ,23 ); + vt =sign_extend32 (R4 ,23 );else - vt =sign_extend (le32_to_cpu (priv->_agn.statistics. + vt =sign_extend32 (le32_to_cpu (priv->_agn.statistics. general.common.temperature),23 );IWL_DEBUG_TEMP (priv,"Calib values R[1-3]: %d %d %d R4: %d\n" ,R1 ,R2 ,R3 , vt);
include/linux/bitops.h +11 -0
@@ -109,6 +109,17 @@ static inline __u8 ror8(__u8 word, unsigned int shift)return (word >> shift) | (word << (8 - shift)); } +/** +* sign_extend32 - sign extend a 32-bit value using specified bit as sign-bit +* @value: value to sign extend +* @index: 0 based bit index (0<=index<32) to sign bit +*/ +static inline __s32 sign_extend32 (__u32 value,int index) +{ +__u8 shift =31 - index; +return (__s32)(value << shift) >> shift; +} +static inline unsigned fls_long (unsigned long l) {if (sizeof (l) ==4 )
Signature · 1 line
1.6.4.4