* [PATCH] wifi: rt2x00: use explicitly signed type for clamping [not found] <202210190108.ESC3pc3D-lkp@intel.com> @ 2022-10-18 20:27 ` Jason A. Donenfeld 2022-10-18 20:40 ` Andy Shevchenko ` (3 more replies) 0 siblings, 4 replies; 20+ messages in thread From: Jason A. Donenfeld @ 2022-10-18 20:27 UTC (permalink / raw) To: linux-wireless, netdev, linux-kernel Cc: Jason A. Donenfeld, Andrew Morton, Andy Shevchenko, Stanislaw Gruszka, Helmut Schaa, Kalle Valo On some platforms, `char` is unsigned, which makes casting -7 to char overflow, which in turn makes the clamping operation bogus. Instead, deal with an explicit `s8` type, so that the comparison is always signed, and return an s8 result from the function as well. Note that this function's result is assigned to a `short`, which is always signed. Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Stanislaw Gruszka <stf_xl@wp.pl> Cc: Helmut Schaa <helmut.schaa@googlemail.com> Cc: Kalle Valo <kvalo@kernel.org> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c index cbbb1a4849cf..e233ef9892b3 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c @@ -4035,23 +4035,23 @@ static void rt2800_iq_calibrate(struct rt2x00_dev *rt2x00dev, int channel) rt2800_bbp_write(rt2x00dev, 159, cal != 0xff ? cal : 0); } -static char rt2800_txpower_to_dev(struct rt2x00_dev *rt2x00dev, +static s8 rt2800_txpower_to_dev(struct rt2x00_dev *rt2x00dev, unsigned int channel, - char txpower) + s8 txpower) { if (rt2x00_rt(rt2x00dev, RT3593) || rt2x00_rt(rt2x00dev, RT3883)) txpower = rt2x00_get_field8(txpower, EEPROM_TXPOWER_ALC); if (channel <= 14) - return clamp_t(char, txpower, MIN_G_TXPOWER, MAX_G_TXPOWER); + return clamp_t(s8, txpower, MIN_G_TXPOWER, MAX_G_TXPOWER); if (rt2x00_rt(rt2x00dev, RT3593) || rt2x00_rt(rt2x00dev, RT3883)) - return clamp_t(char, txpower, MIN_A_TXPOWER_3593, + return clamp_t(s8, txpower, MIN_A_TXPOWER_3593, MAX_A_TXPOWER_3593); else - return clamp_t(char, txpower, MIN_A_TXPOWER, MAX_A_TXPOWER); + return clamp_t(s8, txpower, MIN_A_TXPOWER, MAX_A_TXPOWER); } static void rt3883_bbp_adjust(struct rt2x00_dev *rt2x00dev, -- 2.37.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] wifi: rt2x00: use explicitly signed type for clamping 2022-10-18 20:27 ` [PATCH] wifi: rt2x00: use explicitly signed type for clamping Jason A. Donenfeld @ 2022-10-18 20:40 ` Andy Shevchenko 2022-10-18 20:52 ` Jason A. Donenfeld 2022-10-18 21:07 ` Andrew Morton ` (2 subsequent siblings) 3 siblings, 1 reply; 20+ messages in thread From: Andy Shevchenko @ 2022-10-18 20:40 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-wireless, netdev, linux-kernel, Andrew Morton, Stanislaw Gruszka, Helmut Schaa, Kalle Valo On Tue, Oct 18, 2022 at 02:27:34PM -0600, Jason A. Donenfeld wrote: > On some platforms, `char` is unsigned, which makes casting -7 to char > overflow, which in turn makes the clamping operation bogus. Instead, > deal with an explicit `s8` type, so that the comparison is always > signed, and return an s8 result from the function as well. Note that > this function's result is assigned to a `short`, which is always signed. Why not to use short? See my patch I just sent. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] wifi: rt2x00: use explicitly signed type for clamping 2022-10-18 20:40 ` Andy Shevchenko @ 2022-10-18 20:52 ` Jason A. Donenfeld 2022-10-18 20:57 ` Andy Shevchenko 0 siblings, 1 reply; 20+ messages in thread From: Jason A. Donenfeld @ 2022-10-18 20:52 UTC (permalink / raw) To: Andy Shevchenko Cc: linux-wireless, netdev, linux-kernel, Andrew Morton, Stanislaw Gruszka, Helmut Schaa, Kalle Valo On Tue, Oct 18, 2022 at 11:40:54PM +0300, Andy Shevchenko wrote: > On Tue, Oct 18, 2022 at 02:27:34PM -0600, Jason A. Donenfeld wrote: > > On some platforms, `char` is unsigned, which makes casting -7 to char > > overflow, which in turn makes the clamping operation bogus. Instead, > > deal with an explicit `s8` type, so that the comparison is always > > signed, and return an s8 result from the function as well. Note that > > this function's result is assigned to a `short`, which is always signed. > > Why not to use short? See my patch I just sent. Trying to have the most minimal change here that doesn't rock the boat. I'm not out to rewrite the driver. I don't know the original author's rationales. This patch here is correct and will generate the same code as before on architectures where it wasn't broken. However, if you want your "change the codegen" patch to be taken seriously, you should probably send it to the wireless maintainers like this one, and they can decide. Personally, I don't really care either way. Jason ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] wifi: rt2x00: use explicitly signed type for clamping 2022-10-18 20:52 ` Jason A. Donenfeld @ 2022-10-18 20:57 ` Andy Shevchenko 2022-10-18 20:58 ` Jason A. Donenfeld 0 siblings, 1 reply; 20+ messages in thread From: Andy Shevchenko @ 2022-10-18 20:57 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-wireless, netdev, linux-kernel, Andrew Morton, Stanislaw Gruszka, Helmut Schaa, Kalle Valo On Tue, Oct 18, 2022 at 02:52:43PM -0600, Jason A. Donenfeld wrote: > On Tue, Oct 18, 2022 at 11:40:54PM +0300, Andy Shevchenko wrote: > > On Tue, Oct 18, 2022 at 02:27:34PM -0600, Jason A. Donenfeld wrote: > > > On some platforms, `char` is unsigned, which makes casting -7 to char > > > overflow, which in turn makes the clamping operation bogus. Instead, > > > deal with an explicit `s8` type, so that the comparison is always > > > signed, and return an s8 result from the function as well. Note that > > > this function's result is assigned to a `short`, which is always signed. > > > > Why not to use short? See my patch I just sent. > > Trying to have the most minimal change here that doesn't rock the boat. > I'm not out to rewrite the driver. I don't know the original author's > rationales. This patch here is correct and will generate the same code > as before on architectures where it wasn't broken. > > However, if you want your "change the codegen" patch to be taken > seriously, you should probably send it to the wireless maintainers like > this one, and they can decide. Personally, I don't really care either > way. I have checked the code paths there and I found no evidence that short can't be used. That's why my patch. Okay, I will formally send it to the corresponding maintainers. But if they want, they can always download this thread using `b4` tool and at least comment on it. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] wifi: rt2x00: use explicitly signed type for clamping 2022-10-18 20:57 ` Andy Shevchenko @ 2022-10-18 20:58 ` Jason A. Donenfeld 2022-10-18 21:10 ` Andy Shevchenko 0 siblings, 1 reply; 20+ messages in thread From: Jason A. Donenfeld @ 2022-10-18 20:58 UTC (permalink / raw) To: Andy Shevchenko Cc: linux-wireless, netdev, linux-kernel, Andrew Morton, Stanislaw Gruszka, Helmut Schaa, Kalle Valo On Tue, Oct 18, 2022 at 2:57 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Oct 18, 2022 at 02:52:43PM -0600, Jason A. Donenfeld wrote: > > On Tue, Oct 18, 2022 at 11:40:54PM +0300, Andy Shevchenko wrote: > > > On Tue, Oct 18, 2022 at 02:27:34PM -0600, Jason A. Donenfeld wrote: > > > > On some platforms, `char` is unsigned, which makes casting -7 to char > > > > overflow, which in turn makes the clamping operation bogus. Instead, > > > > deal with an explicit `s8` type, so that the comparison is always > > > > signed, and return an s8 result from the function as well. Note that > > > > this function's result is assigned to a `short`, which is always signed. > > > > > > Why not to use short? See my patch I just sent. > > > > Trying to have the most minimal change here that doesn't rock the boat. > > I'm not out to rewrite the driver. I don't know the original author's > > rationales. This patch here is correct and will generate the same code > > as before on architectures where it wasn't broken. > > > > However, if you want your "change the codegen" patch to be taken > > seriously, you should probably send it to the wireless maintainers like > > this one, and they can decide. Personally, I don't really care either > > way. > > I have checked the code paths there and I found no evidence that short can't be > used. That's why my patch. Do you have a rationale why you want to change codegen? Jason ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] wifi: rt2x00: use explicitly signed type for clamping 2022-10-18 20:58 ` Jason A. Donenfeld @ 2022-10-18 21:10 ` Andy Shevchenko 2022-10-19 8:01 ` David Laight 0 siblings, 1 reply; 20+ messages in thread From: Andy Shevchenko @ 2022-10-18 21:10 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-wireless, netdev, linux-kernel, Andrew Morton, Stanislaw Gruszka, Helmut Schaa, Kalle Valo On Tue, Oct 18, 2022 at 02:58:30PM -0600, Jason A. Donenfeld wrote: > On Tue, Oct 18, 2022 at 2:57 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Oct 18, 2022 at 02:52:43PM -0600, Jason A. Donenfeld wrote: > > > On Tue, Oct 18, 2022 at 11:40:54PM +0300, Andy Shevchenko wrote: > > > > On Tue, Oct 18, 2022 at 02:27:34PM -0600, Jason A. Donenfeld wrote: > > > > > On some platforms, `char` is unsigned, which makes casting -7 to char > > > > > overflow, which in turn makes the clamping operation bogus. Instead, > > > > > deal with an explicit `s8` type, so that the comparison is always > > > > > signed, and return an s8 result from the function as well. Note that > > > > > this function's result is assigned to a `short`, which is always signed. > > > > > > > > Why not to use short? See my patch I just sent. > > > > > > Trying to have the most minimal change here that doesn't rock the boat. > > > I'm not out to rewrite the driver. I don't know the original author's > > > rationales. This patch here is correct and will generate the same code > > > as before on architectures where it wasn't broken. > > > > > > However, if you want your "change the codegen" patch to be taken > > > seriously, you should probably send it to the wireless maintainers like > > > this one, and they can decide. Personally, I don't really care either > > > way. > > > > I have checked the code paths there and I found no evidence that short can't be > > used. That's why my patch. > > Do you have a rationale why you want to change codegen? It's not a hot path as far as I understand and keeping data types aligned seems to me worth it even if codegen is changed. IS it so awful with short? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH] wifi: rt2x00: use explicitly signed type for clamping 2022-10-18 21:10 ` Andy Shevchenko @ 2022-10-19 8:01 ` David Laight 0 siblings, 0 replies; 20+ messages in thread From: David Laight @ 2022-10-19 8:01 UTC (permalink / raw) To: 'Andy Shevchenko', Jason A. Donenfeld Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Andrew Morton, Stanislaw Gruszka, Helmut Schaa, Kalle Valo From: Andy Shevchenko > Sent: 18 October 2022 22:10 .... > It's not a hot path as far as I understand and keeping data types aligned seems > to me worth it even if codegen is changed. IS it so awful with short? (without looking at the generated code) Why is it even short, what is wrong with int? It is extremely unlikely that the code requires any calculation results be masked to 8 (or 16) bits, but using s8 or s16 requires the compiler emit code to mask any calculated values. Remember, all C arithmetic requires promoting the values to (at least) int. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] wifi: rt2x00: use explicitly signed type for clamping 2022-10-18 20:27 ` [PATCH] wifi: rt2x00: use explicitly signed type for clamping Jason A. Donenfeld 2022-10-18 20:40 ` Andy Shevchenko @ 2022-10-18 21:07 ` Andrew Morton 2022-10-19 8:14 ` [PATCH v2] wifi: rt2x00: use explicitly signed or unsigned types Jason A. Donenfeld 2022-10-19 8:52 ` [PATCH] wifi: rt2x00: use explicitly signed type for clamping Stanislaw Gruszka 3 siblings, 0 replies; 20+ messages in thread From: Andrew Morton @ 2022-10-18 21:07 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-wireless, netdev, linux-kernel, Andy Shevchenko, Stanislaw Gruszka, Helmut Schaa, Kalle Valo On Tue, 18 Oct 2022 14:27:34 -0600 "Jason A. Donenfeld" <Jason@zx2c4.com> wrote: > On some platforms, `char` is unsigned, which makes casting -7 to char > overflow, which in turn makes the clamping operation bogus. Instead, > deal with an explicit `s8` type, so that the comparison is always > signed, and return an s8 result from the function as well. Note that > this function's result is assigned to a `short`, which is always signed. Thanks. I'll grab this for now to make -next happier. Stephen will tell us when the patch (or one like it) appears via the wireless tree. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] wifi: rt2x00: use explicitly signed or unsigned types 2022-10-18 20:27 ` [PATCH] wifi: rt2x00: use explicitly signed type for clamping Jason A. Donenfeld 2022-10-18 20:40 ` Andy Shevchenko 2022-10-18 21:07 ` Andrew Morton @ 2022-10-19 8:14 ` Jason A. Donenfeld 2022-10-19 9:00 ` Stanislaw Gruszka 2022-10-19 8:52 ` [PATCH] wifi: rt2x00: use explicitly signed type for clamping Stanislaw Gruszka 3 siblings, 1 reply; 20+ messages in thread From: Jason A. Donenfeld @ 2022-10-19 8:14 UTC (permalink / raw) To: linux-wireless, netdev, linux-kernel Cc: Jason A. Donenfeld, Andrew Morton, Andy Shevchenko, Stanislaw Gruszka, Helmut Schaa, Kalle Valo On some platforms, `char` is unsigned, but this driver, for the most part, assumed it was signed. In other places, it uses `char` to mean an unsigned number, but only in cases when the values are small. And in still other places, `char` is used as a boolean. Put an end to this confusion by declaring explicit types, depending on the context. Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Stanislaw Gruszka <stf_xl@wp.pl> Cc: Helmut Schaa <helmut.schaa@googlemail.com> Cc: Kalle Valo <kvalo@kernel.org> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- .../net/wireless/ralink/rt2x00/rt2400pci.c | 8 +-- .../net/wireless/ralink/rt2x00/rt2400pci.h | 2 +- .../net/wireless/ralink/rt2x00/rt2500pci.c | 8 +-- .../net/wireless/ralink/rt2x00/rt2500pci.h | 2 +- .../net/wireless/ralink/rt2x00/rt2500usb.c | 8 +-- .../net/wireless/ralink/rt2x00/rt2500usb.h | 2 +- .../net/wireless/ralink/rt2x00/rt2800lib.c | 60 +++++++++---------- .../net/wireless/ralink/rt2x00/rt2800lib.h | 8 +-- .../net/wireless/ralink/rt2x00/rt2x00usb.c | 6 +- drivers/net/wireless/ralink/rt2x00/rt61pci.c | 4 +- drivers/net/wireless/ralink/rt2x00/rt61pci.h | 2 +- drivers/net/wireless/ralink/rt2x00/rt73usb.c | 4 +- drivers/net/wireless/ralink/rt2x00/rt73usb.h | 2 +- 13 files changed, 58 insertions(+), 58 deletions(-) diff --git a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c index 273c5eac3362..ddfc16de1b26 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c @@ -1023,9 +1023,9 @@ static int rt2400pci_set_state(struct rt2x00_dev *rt2x00dev, { u32 reg, reg2; unsigned int i; - char put_to_sleep; - char bbp_state; - char rf_state; + bool put_to_sleep; + u8 bbp_state; + u8 rf_state; put_to_sleep = (state != STATE_AWAKE); @@ -1561,7 +1561,7 @@ static int rt2400pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *tx_power; + u8 *tx_power; unsigned int i; /* diff --git a/drivers/net/wireless/ralink/rt2x00/rt2400pci.h b/drivers/net/wireless/ralink/rt2x00/rt2400pci.h index b8187b6de143..979d5fd8babf 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2400pci.h +++ b/drivers/net/wireless/ralink/rt2x00/rt2400pci.h @@ -939,7 +939,7 @@ #define DEFAULT_TXPOWER 39 #define __CLAMP_TX(__txpower) \ - clamp_t(char, (__txpower), MIN_TXPOWER, MAX_TXPOWER) + clamp_t(u8, (__txpower), MIN_TXPOWER, MAX_TXPOWER) #define TXPOWER_FROM_DEV(__txpower) \ ((__CLAMP_TX(__txpower) - MAX_TXPOWER) + MIN_TXPOWER) diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c index 8faa0a80e73a..cd6371e25062 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c @@ -1176,9 +1176,9 @@ static int rt2500pci_set_state(struct rt2x00_dev *rt2x00dev, { u32 reg, reg2; unsigned int i; - char put_to_sleep; - char bbp_state; - char rf_state; + bool put_to_sleep; + u8 bbp_state; + u8 rf_state; put_to_sleep = (state != STATE_AWAKE); @@ -1856,7 +1856,7 @@ static int rt2500pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *tx_power; + u8 *tx_power; unsigned int i; /* diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500pci.h b/drivers/net/wireless/ralink/rt2x00/rt2500pci.h index 7e64aee2a172..ba362675c52c 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2500pci.h +++ b/drivers/net/wireless/ralink/rt2x00/rt2500pci.h @@ -1219,6 +1219,6 @@ (((u8)(__txpower)) > MAX_TXPOWER) ? DEFAULT_TXPOWER : (__txpower) #define TXPOWER_TO_DEV(__txpower) \ - clamp_t(char, __txpower, MIN_TXPOWER, MAX_TXPOWER) + clamp_t(u8, __txpower, MIN_TXPOWER, MAX_TXPOWER) #endif /* RT2500PCI_H */ diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c index bb5ed6630645..4f3b0e6c6256 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c @@ -984,9 +984,9 @@ static int rt2500usb_set_state(struct rt2x00_dev *rt2x00dev, u16 reg; u16 reg2; unsigned int i; - char put_to_sleep; - char bbp_state; - char rf_state; + bool put_to_sleep; + u8 bbp_state; + u8 rf_state; put_to_sleep = (state != STATE_AWAKE); @@ -1663,7 +1663,7 @@ static int rt2500usb_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *tx_power; + u8 *tx_power; unsigned int i; /* diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500usb.h b/drivers/net/wireless/ralink/rt2x00/rt2500usb.h index 0c070288a140..746f0e950b76 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2500usb.h +++ b/drivers/net/wireless/ralink/rt2x00/rt2500usb.h @@ -839,6 +839,6 @@ (((u8)(__txpower)) > MAX_TXPOWER) ? DEFAULT_TXPOWER : (__txpower) #define TXPOWER_TO_DEV(__txpower) \ - clamp_t(char, __txpower, MIN_TXPOWER, MAX_TXPOWER) + clamp_t(u8, __txpower, MIN_TXPOWER, MAX_TXPOWER) #endif /* RT2500USB_H */ diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c index cbbb1a4849cf..eb323fb44c42 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c @@ -3372,10 +3372,10 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev, if (rt2x00_has_cap_bt_coexist(rt2x00dev)) { if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390F)) { /* r55/r59 value array of channel 1~14 */ - static const char r55_bt_rev[] = {0x83, 0x83, + static const u8 r55_bt_rev[] = {0x83, 0x83, 0x83, 0x73, 0x73, 0x63, 0x53, 0x53, 0x53, 0x43, 0x43, 0x43, 0x43, 0x43}; - static const char r59_bt_rev[] = {0x0e, 0x0e, + static const u8 r59_bt_rev[] = {0x0e, 0x0e, 0x0e, 0x0e, 0x0e, 0x0b, 0x0a, 0x09, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07}; @@ -3384,7 +3384,7 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev, rt2800_rfcsr_write(rt2x00dev, 59, r59_bt_rev[idx]); } else { - static const char r59_bt[] = {0x8b, 0x8b, 0x8b, + static const u8 r59_bt[] = {0x8b, 0x8b, 0x8b, 0x8b, 0x8b, 0x8b, 0x8b, 0x8a, 0x89, 0x88, 0x88, 0x86, 0x85, 0x84}; @@ -3392,10 +3392,10 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev, } } else { if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390F)) { - static const char r55_nonbt_rev[] = {0x23, 0x23, + static const u8 r55_nonbt_rev[] = {0x23, 0x23, 0x23, 0x23, 0x13, 0x13, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03}; - static const char r59_nonbt_rev[] = {0x07, 0x07, + static const u8 r59_nonbt_rev[] = {0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x06, 0x05, 0x04, 0x04}; @@ -3406,14 +3406,14 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev, } else if (rt2x00_rt(rt2x00dev, RT5390) || rt2x00_rt(rt2x00dev, RT5392) || rt2x00_rt(rt2x00dev, RT6352)) { - static const char r59_non_bt[] = {0x8f, 0x8f, + static const s8 r59_non_bt[] = {0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8d, 0x8a, 0x88, 0x88, 0x87, 0x87, 0x86}; rt2800_rfcsr_write(rt2x00dev, 59, r59_non_bt[idx]); } else if (rt2x00_rt(rt2x00dev, RT5350)) { - static const char r59_non_bt[] = {0x0b, 0x0b, + static const s8 r59_non_bt[] = {0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0a, 0x0a, 0x09, 0x08, 0x07, 0x07, 0x06}; @@ -4035,23 +4035,23 @@ static void rt2800_iq_calibrate(struct rt2x00_dev *rt2x00dev, int channel) rt2800_bbp_write(rt2x00dev, 159, cal != 0xff ? cal : 0); } -static char rt2800_txpower_to_dev(struct rt2x00_dev *rt2x00dev, +static s8 rt2800_txpower_to_dev(struct rt2x00_dev *rt2x00dev, unsigned int channel, - char txpower) + s8 txpower) { if (rt2x00_rt(rt2x00dev, RT3593) || rt2x00_rt(rt2x00dev, RT3883)) txpower = rt2x00_get_field8(txpower, EEPROM_TXPOWER_ALC); if (channel <= 14) - return clamp_t(char, txpower, MIN_G_TXPOWER, MAX_G_TXPOWER); + return clamp_t(s8, txpower, MIN_G_TXPOWER, MAX_G_TXPOWER); if (rt2x00_rt(rt2x00dev, RT3593) || rt2x00_rt(rt2x00dev, RT3883)) - return clamp_t(char, txpower, MIN_A_TXPOWER_3593, + return clamp_t(s8, txpower, MIN_A_TXPOWER_3593, MAX_A_TXPOWER_3593); else - return clamp_t(char, txpower, MIN_A_TXPOWER, MAX_A_TXPOWER); + return clamp_t(s8, txpower, MIN_A_TXPOWER, MAX_A_TXPOWER); } static void rt3883_bbp_adjust(struct rt2x00_dev *rt2x00dev, @@ -8530,7 +8530,7 @@ static void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev) u8 bytevalue = 0; int rcalcode; u8 r_cal_code = 0; - char d1 = 0, d2 = 0; + s8 d1 = 0, d2 = 0; u8 rfvalue; u32 MAC_RF_BYPASS0, MAC_RF_CONTROL0, MAC_PWR_PIN_CFG; u32 maccfg; @@ -8591,7 +8591,7 @@ static void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev) if (bytevalue > 128) d1 = bytevalue - 256; else - d1 = (char)bytevalue; + d1 = (s8)bytevalue; rt2800_bbp_write(rt2x00dev, 22, 0x0); rt2800_rfcsr_write_bank(rt2x00dev, 0, 35, 0x01); @@ -8601,7 +8601,7 @@ static void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev) if (bytevalue > 128) d2 = bytevalue - 256; else - d2 = (char)bytevalue; + d2 = (s8)bytevalue; rt2800_bbp_write(rt2x00dev, 22, 0x0); rcalcode = rt2800_calcrcalibrationcode(rt2x00dev, d1, d2); @@ -8703,7 +8703,7 @@ static void rt2800_rxdcoc_calibration(struct rt2x00_dev *rt2x00dev) static u32 rt2800_do_sqrt_accumulation(u32 si) { u32 root, root_pre, bit; - char i; + s8 i; bit = 1 << 15; root = 0; @@ -9330,11 +9330,11 @@ static void rt2800_loft_search(struct rt2x00_dev *rt2x00dev, u8 ch_idx, u8 alc_idx, u8 dc_result[][RF_ALC_NUM][2]) { u32 p0 = 0, p1 = 0, pf = 0; - char idx0 = 0, idx1 = 0; + s8 idx0 = 0, idx1 = 0; u8 idxf[] = {0x00, 0x00}; u8 ibit = 0x20; u8 iorq; - char bidx; + s8 bidx; rt2800_bbp_write(rt2x00dev, 158, 0xb0); rt2800_bbp_write(rt2x00dev, 159, 0x80); @@ -9384,17 +9384,17 @@ static void rt2800_loft_search(struct rt2x00_dev *rt2x00dev, u8 ch_idx, static void rt2800_iq_search(struct rt2x00_dev *rt2x00dev, u8 ch_idx, u8 *ges, u8 *pes) { u32 p0 = 0, p1 = 0, pf = 0; - char perr = 0, gerr = 0, iq_err = 0; - char pef = 0, gef = 0; - char psta, pend; - char gsta, gend; + s8 perr = 0, gerr = 0, iq_err = 0; + s8 pef = 0, gef = 0; + s8 psta, pend; + s8 gsta, gend; u8 ibit = 0x20; u8 first_search = 0x00, touch_neg_max = 0x00; - char idx0 = 0, idx1 = 0; + s8 idx0 = 0, idx1 = 0; u8 gop; u8 bbp = 0; - char bidx; + s8 bidx; for (bidx = 5; bidx >= 1; bidx--) { for (gop = 0; gop < 2; gop++) { @@ -10043,11 +10043,11 @@ static int rt2800_rf_lp_config(struct rt2x00_dev *rt2x00dev, bool btxcal) return 0; } -static char rt2800_lp_tx_filter_bw_cal(struct rt2x00_dev *rt2x00dev) +static s8 rt2800_lp_tx_filter_bw_cal(struct rt2x00_dev *rt2x00dev) { unsigned int cnt; u8 bbp_val; - char cal_val; + s8 cal_val; rt2800_bbp_dcoc_write(rt2x00dev, 0, 0x82); @@ -10079,7 +10079,7 @@ static void rt2800_bw_filter_calibration(struct rt2x00_dev *rt2x00dev, u8 rx_filter_target_20m = 0x27, rx_filter_target_40m = 0x31; int loop = 0, is_ht40, cnt; u8 bbp_val, rf_val; - char cal_r32_init, cal_r32_val, cal_diff; + s8 cal_r32_init, cal_r32_val, cal_diff; u8 saverfb5r00, saverfb5r01, saverfb5r03, saverfb5r04, saverfb5r05; u8 saverfb5r06, saverfb5r07; u8 saverfb5r08, saverfb5r17, saverfb5r18, saverfb5r19, saverfb5r20; @@ -11550,9 +11550,9 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *default_power1; - char *default_power2; - char *default_power3; + s8 *default_power1; + s8 *default_power2; + s8 *default_power3; unsigned int i, tx_chains, rx_chains; u32 reg; diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.h b/drivers/net/wireless/ralink/rt2x00/rt2800lib.h index 3cbef77b4bd3..194de676df8f 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.h +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.h @@ -32,10 +32,10 @@ struct rf_reg_pair { struct rt2800_drv_data { u8 calibration_bw20; u8 calibration_bw40; - char rx_calibration_bw20; - char rx_calibration_bw40; - char tx_calibration_bw20; - char tx_calibration_bw40; + s8 rx_calibration_bw20; + s8 rx_calibration_bw40; + s8 tx_calibration_bw20; + s8 tx_calibration_bw40; u8 bbp25; u8 bbp26; u8 txmixer_gain_24g; diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c index 0827bc860bf8..8fd22c69855f 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c @@ -117,12 +117,12 @@ int rt2x00usb_vendor_request_buff(struct rt2x00_dev *rt2x00dev, const u16 buffer_length) { int status = 0; - unsigned char *tb; + u8 *tb; u16 off, len, bsize; mutex_lock(&rt2x00dev->csr_mutex); - tb = (char *)buffer; + tb = (u8 *)buffer; off = offset; len = buffer_length; while (len && !status) { @@ -215,7 +215,7 @@ void rt2x00usb_register_read_async(struct rt2x00_dev *rt2x00dev, rd->cr.wLength = cpu_to_le16(sizeof(u32)); usb_fill_control_urb(urb, usb_dev, usb_rcvctrlpipe(usb_dev, 0), - (unsigned char *)(&rd->cr), &rd->reg, sizeof(rd->reg), + (u8 *)(&rd->cr), &rd->reg, sizeof(rd->reg), rt2x00usb_register_read_async_cb, rd); usb_anchor_urb(urb, rt2x00dev->anchor); if (usb_submit_urb(urb, GFP_ATOMIC) < 0) { diff --git a/drivers/net/wireless/ralink/rt2x00/rt61pci.c b/drivers/net/wireless/ralink/rt2x00/rt61pci.c index d92f9eb07dc9..81db7f57c7e4 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt61pci.c +++ b/drivers/net/wireless/ralink/rt2x00/rt61pci.c @@ -1709,7 +1709,7 @@ static int rt61pci_set_state(struct rt2x00_dev *rt2x00dev, enum dev_state state) { u32 reg, reg2; unsigned int i; - char put_to_sleep; + bool put_to_sleep; put_to_sleep = (state != STATE_AWAKE); @@ -2656,7 +2656,7 @@ static int rt61pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *tx_power; + u8 *tx_power; unsigned int i; /* diff --git a/drivers/net/wireless/ralink/rt2x00/rt61pci.h b/drivers/net/wireless/ralink/rt2x00/rt61pci.h index 5f208ad509bd..d72d0ffd1127 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt61pci.h +++ b/drivers/net/wireless/ralink/rt2x00/rt61pci.h @@ -1484,6 +1484,6 @@ struct hw_pairwise_ta_entry { (((u8)(__txpower)) > MAX_TXPOWER) ? DEFAULT_TXPOWER : (__txpower) #define TXPOWER_TO_DEV(__txpower) \ - clamp_t(char, __txpower, MIN_TXPOWER, MAX_TXPOWER) + clamp_t(u8, __txpower, MIN_TXPOWER, MAX_TXPOWER) #endif /* RT61PCI_H */ diff --git a/drivers/net/wireless/ralink/rt2x00/rt73usb.c b/drivers/net/wireless/ralink/rt2x00/rt73usb.c index e3269fd7c59e..861035444374 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt73usb.c +++ b/drivers/net/wireless/ralink/rt2x00/rt73usb.c @@ -1378,7 +1378,7 @@ static int rt73usb_set_state(struct rt2x00_dev *rt2x00dev, enum dev_state state) { u32 reg, reg2; unsigned int i; - char put_to_sleep; + bool put_to_sleep; put_to_sleep = (state != STATE_AWAKE); @@ -2090,7 +2090,7 @@ static int rt73usb_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *tx_power; + u8 *tx_power; unsigned int i; /* diff --git a/drivers/net/wireless/ralink/rt2x00/rt73usb.h b/drivers/net/wireless/ralink/rt2x00/rt73usb.h index 1b56d285c34b..bb0a68516c08 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt73usb.h +++ b/drivers/net/wireless/ralink/rt2x00/rt73usb.h @@ -1063,6 +1063,6 @@ struct hw_pairwise_ta_entry { (((u8)(__txpower)) > MAX_TXPOWER) ? DEFAULT_TXPOWER : (__txpower) #define TXPOWER_TO_DEV(__txpower) \ - clamp_t(char, __txpower, MIN_TXPOWER, MAX_TXPOWER) + clamp_t(u8, __txpower, MIN_TXPOWER, MAX_TXPOWER) #endif /* RT73USB_H */ -- 2.38.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] wifi: rt2x00: use explicitly signed or unsigned types 2022-10-19 8:14 ` [PATCH v2] wifi: rt2x00: use explicitly signed or unsigned types Jason A. Donenfeld @ 2022-10-19 9:00 ` Stanislaw Gruszka 2022-10-19 15:54 ` Jason A. Donenfeld 0 siblings, 1 reply; 20+ messages in thread From: Stanislaw Gruszka @ 2022-10-19 9:00 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-wireless, netdev, linux-kernel, Andrew Morton, Andy Shevchenko, Helmut Schaa, Kalle Valo On Wed, Oct 19, 2022 at 02:14:17AM -0600, Jason A. Donenfeld wrote: > On some platforms, `char` is unsigned, but this driver, for the most > part, assumed it was signed. In other places, it uses `char` to mean an > unsigned number, but only in cases when the values are small. And in > still other places, `char` is used as a boolean. Put an end to this > confusion by declaring explicit types, depending on the context. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Stanislaw Gruszka <stf_xl@wp.pl> > Cc: Helmut Schaa <helmut.schaa@googlemail.com> > Cc: Kalle Valo <kvalo@kernel.org> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> <snip> > @@ -3406,14 +3406,14 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev, > } else if (rt2x00_rt(rt2x00dev, RT5390) || > rt2x00_rt(rt2x00dev, RT5392) || > rt2x00_rt(rt2x00dev, RT6352)) { > - static const char r59_non_bt[] = {0x8f, 0x8f, > + static const s8 r59_non_bt[] = {0x8f, 0x8f, > 0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8d, > 0x8a, 0x88, 0x88, 0x87, 0x87, 0x86}; > > rt2800_rfcsr_write(rt2x00dev, 59, > r59_non_bt[idx]); > } else if (rt2x00_rt(rt2x00dev, RT5350)) { > - static const char r59_non_bt[] = {0x0b, 0x0b, > + static const s8 r59_non_bt[] = {0x0b, 0x0b, > 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0a, > 0x0a, 0x09, 0x08, 0x07, 0x07, 0x06}; Please make those two tables u8 as well. Regards Stanislaw ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] wifi: rt2x00: use explicitly signed or unsigned types 2022-10-19 9:00 ` Stanislaw Gruszka @ 2022-10-19 15:54 ` Jason A. Donenfeld 2022-10-19 15:55 ` [PATCH v3] " Jason A. Donenfeld 0 siblings, 1 reply; 20+ messages in thread From: Jason A. Donenfeld @ 2022-10-19 15:54 UTC (permalink / raw) To: Stanislaw Gruszka Cc: linux-wireless, netdev, linux-kernel, Andrew Morton, Andy Shevchenko, Helmut Schaa, Kalle Valo On Wed, Oct 19, 2022 at 3:00 AM Stanislaw Gruszka <stf_xl@wp.pl> wrote: > > On Wed, Oct 19, 2022 at 02:14:17AM -0600, Jason A. Donenfeld wrote: > > On some platforms, `char` is unsigned, but this driver, for the most > > part, assumed it was signed. In other places, it uses `char` to mean an > > unsigned number, but only in cases when the values are small. And in > > still other places, `char` is used as a boolean. Put an end to this > > confusion by declaring explicit types, depending on the context. > > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Cc: Stanislaw Gruszka <stf_xl@wp.pl> > > Cc: Helmut Schaa <helmut.schaa@googlemail.com> > > Cc: Kalle Valo <kvalo@kernel.org> > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > <snip> > > > @@ -3406,14 +3406,14 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev, > > } else if (rt2x00_rt(rt2x00dev, RT5390) || > > rt2x00_rt(rt2x00dev, RT5392) || > > rt2x00_rt(rt2x00dev, RT6352)) { > > - static const char r59_non_bt[] = {0x8f, 0x8f, > > + static const s8 r59_non_bt[] = {0x8f, 0x8f, > > 0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8d, > > 0x8a, 0x88, 0x88, 0x87, 0x87, 0x86}; > > > > rt2800_rfcsr_write(rt2x00dev, 59, > > r59_non_bt[idx]); > > } else if (rt2x00_rt(rt2x00dev, RT5350)) { > > - static const char r59_non_bt[] = {0x0b, 0x0b, > > + static const s8 r59_non_bt[] = {0x0b, 0x0b, > > 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0a, > > 0x0a, 0x09, 0x08, 0x07, 0x07, 0x06}; > > Please make those two tables u8 as well. Nice catch. Will do. Jason ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] wifi: rt2x00: use explicitly signed or unsigned types 2022-10-19 15:54 ` Jason A. Donenfeld @ 2022-10-19 15:55 ` Jason A. Donenfeld 2022-10-20 10:29 ` Stanislaw Gruszka ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Jason A. Donenfeld @ 2022-10-19 15:55 UTC (permalink / raw) To: linux-wireless, netdev, linux-kernel Cc: Jason A. Donenfeld, Andrew Morton, Andy Shevchenko, Stanislaw Gruszka, Helmut Schaa, Kalle Valo On some platforms, `char` is unsigned, but this driver, for the most part, assumed it was signed. In other places, it uses `char` to mean an unsigned number, but only in cases when the values are small. And in still other places, `char` is used as a boolean. Put an end to this confusion by declaring explicit types, depending on the context. Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Stanislaw Gruszka <stf_xl@wp.pl> Cc: Helmut Schaa <helmut.schaa@googlemail.com> Cc: Kalle Valo <kvalo@kernel.org> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- .../net/wireless/ralink/rt2x00/rt2400pci.c | 8 +-- .../net/wireless/ralink/rt2x00/rt2400pci.h | 2 +- .../net/wireless/ralink/rt2x00/rt2500pci.c | 8 +-- .../net/wireless/ralink/rt2x00/rt2500pci.h | 2 +- .../net/wireless/ralink/rt2x00/rt2500usb.c | 8 +-- .../net/wireless/ralink/rt2x00/rt2500usb.h | 2 +- .../net/wireless/ralink/rt2x00/rt2800lib.c | 60 +++++++++---------- .../net/wireless/ralink/rt2x00/rt2800lib.h | 8 +-- .../net/wireless/ralink/rt2x00/rt2x00usb.c | 6 +- drivers/net/wireless/ralink/rt2x00/rt61pci.c | 4 +- drivers/net/wireless/ralink/rt2x00/rt61pci.h | 2 +- drivers/net/wireless/ralink/rt2x00/rt73usb.c | 4 +- drivers/net/wireless/ralink/rt2x00/rt73usb.h | 2 +- 13 files changed, 58 insertions(+), 58 deletions(-) diff --git a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c index 273c5eac3362..ddfc16de1b26 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c @@ -1023,9 +1023,9 @@ static int rt2400pci_set_state(struct rt2x00_dev *rt2x00dev, { u32 reg, reg2; unsigned int i; - char put_to_sleep; - char bbp_state; - char rf_state; + bool put_to_sleep; + u8 bbp_state; + u8 rf_state; put_to_sleep = (state != STATE_AWAKE); @@ -1561,7 +1561,7 @@ static int rt2400pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *tx_power; + u8 *tx_power; unsigned int i; /* diff --git a/drivers/net/wireless/ralink/rt2x00/rt2400pci.h b/drivers/net/wireless/ralink/rt2x00/rt2400pci.h index b8187b6de143..979d5fd8babf 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2400pci.h +++ b/drivers/net/wireless/ralink/rt2x00/rt2400pci.h @@ -939,7 +939,7 @@ #define DEFAULT_TXPOWER 39 #define __CLAMP_TX(__txpower) \ - clamp_t(char, (__txpower), MIN_TXPOWER, MAX_TXPOWER) + clamp_t(u8, (__txpower), MIN_TXPOWER, MAX_TXPOWER) #define TXPOWER_FROM_DEV(__txpower) \ ((__CLAMP_TX(__txpower) - MAX_TXPOWER) + MIN_TXPOWER) diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c index 8faa0a80e73a..cd6371e25062 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c @@ -1176,9 +1176,9 @@ static int rt2500pci_set_state(struct rt2x00_dev *rt2x00dev, { u32 reg, reg2; unsigned int i; - char put_to_sleep; - char bbp_state; - char rf_state; + bool put_to_sleep; + u8 bbp_state; + u8 rf_state; put_to_sleep = (state != STATE_AWAKE); @@ -1856,7 +1856,7 @@ static int rt2500pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *tx_power; + u8 *tx_power; unsigned int i; /* diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500pci.h b/drivers/net/wireless/ralink/rt2x00/rt2500pci.h index 7e64aee2a172..ba362675c52c 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2500pci.h +++ b/drivers/net/wireless/ralink/rt2x00/rt2500pci.h @@ -1219,6 +1219,6 @@ (((u8)(__txpower)) > MAX_TXPOWER) ? DEFAULT_TXPOWER : (__txpower) #define TXPOWER_TO_DEV(__txpower) \ - clamp_t(char, __txpower, MIN_TXPOWER, MAX_TXPOWER) + clamp_t(u8, __txpower, MIN_TXPOWER, MAX_TXPOWER) #endif /* RT2500PCI_H */ diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c index bb5ed6630645..4f3b0e6c6256 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c @@ -984,9 +984,9 @@ static int rt2500usb_set_state(struct rt2x00_dev *rt2x00dev, u16 reg; u16 reg2; unsigned int i; - char put_to_sleep; - char bbp_state; - char rf_state; + bool put_to_sleep; + u8 bbp_state; + u8 rf_state; put_to_sleep = (state != STATE_AWAKE); @@ -1663,7 +1663,7 @@ static int rt2500usb_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *tx_power; + u8 *tx_power; unsigned int i; /* diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500usb.h b/drivers/net/wireless/ralink/rt2x00/rt2500usb.h index 0c070288a140..746f0e950b76 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2500usb.h +++ b/drivers/net/wireless/ralink/rt2x00/rt2500usb.h @@ -839,6 +839,6 @@ (((u8)(__txpower)) > MAX_TXPOWER) ? DEFAULT_TXPOWER : (__txpower) #define TXPOWER_TO_DEV(__txpower) \ - clamp_t(char, __txpower, MIN_TXPOWER, MAX_TXPOWER) + clamp_t(u8, __txpower, MIN_TXPOWER, MAX_TXPOWER) #endif /* RT2500USB_H */ diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c index cbbb1a4849cf..12b700c7b9c3 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c @@ -3372,10 +3372,10 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev, if (rt2x00_has_cap_bt_coexist(rt2x00dev)) { if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390F)) { /* r55/r59 value array of channel 1~14 */ - static const char r55_bt_rev[] = {0x83, 0x83, + static const u8 r55_bt_rev[] = {0x83, 0x83, 0x83, 0x73, 0x73, 0x63, 0x53, 0x53, 0x53, 0x43, 0x43, 0x43, 0x43, 0x43}; - static const char r59_bt_rev[] = {0x0e, 0x0e, + static const u8 r59_bt_rev[] = {0x0e, 0x0e, 0x0e, 0x0e, 0x0e, 0x0b, 0x0a, 0x09, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07}; @@ -3384,7 +3384,7 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev, rt2800_rfcsr_write(rt2x00dev, 59, r59_bt_rev[idx]); } else { - static const char r59_bt[] = {0x8b, 0x8b, 0x8b, + static const u8 r59_bt[] = {0x8b, 0x8b, 0x8b, 0x8b, 0x8b, 0x8b, 0x8b, 0x8a, 0x89, 0x88, 0x88, 0x86, 0x85, 0x84}; @@ -3392,10 +3392,10 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev, } } else { if (rt2x00_rt_rev_gte(rt2x00dev, RT5390, REV_RT5390F)) { - static const char r55_nonbt_rev[] = {0x23, 0x23, + static const u8 r55_nonbt_rev[] = {0x23, 0x23, 0x23, 0x23, 0x13, 0x13, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03, 0x03}; - static const char r59_nonbt_rev[] = {0x07, 0x07, + static const u8 r59_nonbt_rev[] = {0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x07, 0x06, 0x05, 0x04, 0x04}; @@ -3406,14 +3406,14 @@ static void rt2800_config_channel_rf53xx(struct rt2x00_dev *rt2x00dev, } else if (rt2x00_rt(rt2x00dev, RT5390) || rt2x00_rt(rt2x00dev, RT5392) || rt2x00_rt(rt2x00dev, RT6352)) { - static const char r59_non_bt[] = {0x8f, 0x8f, + static const u8 r59_non_bt[] = {0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8f, 0x8d, 0x8a, 0x88, 0x88, 0x87, 0x87, 0x86}; rt2800_rfcsr_write(rt2x00dev, 59, r59_non_bt[idx]); } else if (rt2x00_rt(rt2x00dev, RT5350)) { - static const char r59_non_bt[] = {0x0b, 0x0b, + static const u8 r59_non_bt[] = {0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0a, 0x0a, 0x09, 0x08, 0x07, 0x07, 0x06}; @@ -4035,23 +4035,23 @@ static void rt2800_iq_calibrate(struct rt2x00_dev *rt2x00dev, int channel) rt2800_bbp_write(rt2x00dev, 159, cal != 0xff ? cal : 0); } -static char rt2800_txpower_to_dev(struct rt2x00_dev *rt2x00dev, +static s8 rt2800_txpower_to_dev(struct rt2x00_dev *rt2x00dev, unsigned int channel, - char txpower) + s8 txpower) { if (rt2x00_rt(rt2x00dev, RT3593) || rt2x00_rt(rt2x00dev, RT3883)) txpower = rt2x00_get_field8(txpower, EEPROM_TXPOWER_ALC); if (channel <= 14) - return clamp_t(char, txpower, MIN_G_TXPOWER, MAX_G_TXPOWER); + return clamp_t(s8, txpower, MIN_G_TXPOWER, MAX_G_TXPOWER); if (rt2x00_rt(rt2x00dev, RT3593) || rt2x00_rt(rt2x00dev, RT3883)) - return clamp_t(char, txpower, MIN_A_TXPOWER_3593, + return clamp_t(s8, txpower, MIN_A_TXPOWER_3593, MAX_A_TXPOWER_3593); else - return clamp_t(char, txpower, MIN_A_TXPOWER, MAX_A_TXPOWER); + return clamp_t(s8, txpower, MIN_A_TXPOWER, MAX_A_TXPOWER); } static void rt3883_bbp_adjust(struct rt2x00_dev *rt2x00dev, @@ -8530,7 +8530,7 @@ static void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev) u8 bytevalue = 0; int rcalcode; u8 r_cal_code = 0; - char d1 = 0, d2 = 0; + s8 d1 = 0, d2 = 0; u8 rfvalue; u32 MAC_RF_BYPASS0, MAC_RF_CONTROL0, MAC_PWR_PIN_CFG; u32 maccfg; @@ -8591,7 +8591,7 @@ static void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev) if (bytevalue > 128) d1 = bytevalue - 256; else - d1 = (char)bytevalue; + d1 = (s8)bytevalue; rt2800_bbp_write(rt2x00dev, 22, 0x0); rt2800_rfcsr_write_bank(rt2x00dev, 0, 35, 0x01); @@ -8601,7 +8601,7 @@ static void rt2800_r_calibration(struct rt2x00_dev *rt2x00dev) if (bytevalue > 128) d2 = bytevalue - 256; else - d2 = (char)bytevalue; + d2 = (s8)bytevalue; rt2800_bbp_write(rt2x00dev, 22, 0x0); rcalcode = rt2800_calcrcalibrationcode(rt2x00dev, d1, d2); @@ -8703,7 +8703,7 @@ static void rt2800_rxdcoc_calibration(struct rt2x00_dev *rt2x00dev) static u32 rt2800_do_sqrt_accumulation(u32 si) { u32 root, root_pre, bit; - char i; + s8 i; bit = 1 << 15; root = 0; @@ -9330,11 +9330,11 @@ static void rt2800_loft_search(struct rt2x00_dev *rt2x00dev, u8 ch_idx, u8 alc_idx, u8 dc_result[][RF_ALC_NUM][2]) { u32 p0 = 0, p1 = 0, pf = 0; - char idx0 = 0, idx1 = 0; + s8 idx0 = 0, idx1 = 0; u8 idxf[] = {0x00, 0x00}; u8 ibit = 0x20; u8 iorq; - char bidx; + s8 bidx; rt2800_bbp_write(rt2x00dev, 158, 0xb0); rt2800_bbp_write(rt2x00dev, 159, 0x80); @@ -9384,17 +9384,17 @@ static void rt2800_loft_search(struct rt2x00_dev *rt2x00dev, u8 ch_idx, static void rt2800_iq_search(struct rt2x00_dev *rt2x00dev, u8 ch_idx, u8 *ges, u8 *pes) { u32 p0 = 0, p1 = 0, pf = 0; - char perr = 0, gerr = 0, iq_err = 0; - char pef = 0, gef = 0; - char psta, pend; - char gsta, gend; + s8 perr = 0, gerr = 0, iq_err = 0; + s8 pef = 0, gef = 0; + s8 psta, pend; + s8 gsta, gend; u8 ibit = 0x20; u8 first_search = 0x00, touch_neg_max = 0x00; - char idx0 = 0, idx1 = 0; + s8 idx0 = 0, idx1 = 0; u8 gop; u8 bbp = 0; - char bidx; + s8 bidx; for (bidx = 5; bidx >= 1; bidx--) { for (gop = 0; gop < 2; gop++) { @@ -10043,11 +10043,11 @@ static int rt2800_rf_lp_config(struct rt2x00_dev *rt2x00dev, bool btxcal) return 0; } -static char rt2800_lp_tx_filter_bw_cal(struct rt2x00_dev *rt2x00dev) +static s8 rt2800_lp_tx_filter_bw_cal(struct rt2x00_dev *rt2x00dev) { unsigned int cnt; u8 bbp_val; - char cal_val; + s8 cal_val; rt2800_bbp_dcoc_write(rt2x00dev, 0, 0x82); @@ -10079,7 +10079,7 @@ static void rt2800_bw_filter_calibration(struct rt2x00_dev *rt2x00dev, u8 rx_filter_target_20m = 0x27, rx_filter_target_40m = 0x31; int loop = 0, is_ht40, cnt; u8 bbp_val, rf_val; - char cal_r32_init, cal_r32_val, cal_diff; + s8 cal_r32_init, cal_r32_val, cal_diff; u8 saverfb5r00, saverfb5r01, saverfb5r03, saverfb5r04, saverfb5r05; u8 saverfb5r06, saverfb5r07; u8 saverfb5r08, saverfb5r17, saverfb5r18, saverfb5r19, saverfb5r20; @@ -11550,9 +11550,9 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *default_power1; - char *default_power2; - char *default_power3; + s8 *default_power1; + s8 *default_power2; + s8 *default_power3; unsigned int i, tx_chains, rx_chains; u32 reg; diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.h b/drivers/net/wireless/ralink/rt2x00/rt2800lib.h index 3cbef77b4bd3..194de676df8f 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.h +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.h @@ -32,10 +32,10 @@ struct rf_reg_pair { struct rt2800_drv_data { u8 calibration_bw20; u8 calibration_bw40; - char rx_calibration_bw20; - char rx_calibration_bw40; - char tx_calibration_bw20; - char tx_calibration_bw40; + s8 rx_calibration_bw20; + s8 rx_calibration_bw40; + s8 tx_calibration_bw20; + s8 tx_calibration_bw40; u8 bbp25; u8 bbp26; u8 txmixer_gain_24g; diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c index 0827bc860bf8..8fd22c69855f 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00usb.c @@ -117,12 +117,12 @@ int rt2x00usb_vendor_request_buff(struct rt2x00_dev *rt2x00dev, const u16 buffer_length) { int status = 0; - unsigned char *tb; + u8 *tb; u16 off, len, bsize; mutex_lock(&rt2x00dev->csr_mutex); - tb = (char *)buffer; + tb = (u8 *)buffer; off = offset; len = buffer_length; while (len && !status) { @@ -215,7 +215,7 @@ void rt2x00usb_register_read_async(struct rt2x00_dev *rt2x00dev, rd->cr.wLength = cpu_to_le16(sizeof(u32)); usb_fill_control_urb(urb, usb_dev, usb_rcvctrlpipe(usb_dev, 0), - (unsigned char *)(&rd->cr), &rd->reg, sizeof(rd->reg), + (u8 *)(&rd->cr), &rd->reg, sizeof(rd->reg), rt2x00usb_register_read_async_cb, rd); usb_anchor_urb(urb, rt2x00dev->anchor); if (usb_submit_urb(urb, GFP_ATOMIC) < 0) { diff --git a/drivers/net/wireless/ralink/rt2x00/rt61pci.c b/drivers/net/wireless/ralink/rt2x00/rt61pci.c index d92f9eb07dc9..81db7f57c7e4 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt61pci.c +++ b/drivers/net/wireless/ralink/rt2x00/rt61pci.c @@ -1709,7 +1709,7 @@ static int rt61pci_set_state(struct rt2x00_dev *rt2x00dev, enum dev_state state) { u32 reg, reg2; unsigned int i; - char put_to_sleep; + bool put_to_sleep; put_to_sleep = (state != STATE_AWAKE); @@ -2656,7 +2656,7 @@ static int rt61pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *tx_power; + u8 *tx_power; unsigned int i; /* diff --git a/drivers/net/wireless/ralink/rt2x00/rt61pci.h b/drivers/net/wireless/ralink/rt2x00/rt61pci.h index 5f208ad509bd..d72d0ffd1127 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt61pci.h +++ b/drivers/net/wireless/ralink/rt2x00/rt61pci.h @@ -1484,6 +1484,6 @@ struct hw_pairwise_ta_entry { (((u8)(__txpower)) > MAX_TXPOWER) ? DEFAULT_TXPOWER : (__txpower) #define TXPOWER_TO_DEV(__txpower) \ - clamp_t(char, __txpower, MIN_TXPOWER, MAX_TXPOWER) + clamp_t(u8, __txpower, MIN_TXPOWER, MAX_TXPOWER) #endif /* RT61PCI_H */ diff --git a/drivers/net/wireless/ralink/rt2x00/rt73usb.c b/drivers/net/wireless/ralink/rt2x00/rt73usb.c index e3269fd7c59e..861035444374 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt73usb.c +++ b/drivers/net/wireless/ralink/rt2x00/rt73usb.c @@ -1378,7 +1378,7 @@ static int rt73usb_set_state(struct rt2x00_dev *rt2x00dev, enum dev_state state) { u32 reg, reg2; unsigned int i; - char put_to_sleep; + bool put_to_sleep; put_to_sleep = (state != STATE_AWAKE); @@ -2090,7 +2090,7 @@ static int rt73usb_probe_hw_mode(struct rt2x00_dev *rt2x00dev) { struct hw_mode_spec *spec = &rt2x00dev->spec; struct channel_info *info; - char *tx_power; + u8 *tx_power; unsigned int i; /* diff --git a/drivers/net/wireless/ralink/rt2x00/rt73usb.h b/drivers/net/wireless/ralink/rt2x00/rt73usb.h index 1b56d285c34b..bb0a68516c08 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt73usb.h +++ b/drivers/net/wireless/ralink/rt2x00/rt73usb.h @@ -1063,6 +1063,6 @@ struct hw_pairwise_ta_entry { (((u8)(__txpower)) > MAX_TXPOWER) ? DEFAULT_TXPOWER : (__txpower) #define TXPOWER_TO_DEV(__txpower) \ - clamp_t(char, __txpower, MIN_TXPOWER, MAX_TXPOWER) + clamp_t(u8, __txpower, MIN_TXPOWER, MAX_TXPOWER) #endif /* RT73USB_H */ -- 2.38.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3] wifi: rt2x00: use explicitly signed or unsigned types 2022-10-19 15:55 ` [PATCH v3] " Jason A. Donenfeld @ 2022-10-20 10:29 ` Stanislaw Gruszka 2022-10-20 10:36 ` Kalle Valo 2022-10-21 6:48 ` Kalle Valo 2022-10-21 7:00 ` Kalle Valo 2 siblings, 1 reply; 20+ messages in thread From: Stanislaw Gruszka @ 2022-10-20 10:29 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-wireless, netdev, linux-kernel, Andrew Morton, Andy Shevchenko, Helmut Schaa, Kalle Valo On Wed, Oct 19, 2022 at 09:55:41AM -0600, Jason A. Donenfeld wrote: > On some platforms, `char` is unsigned, but this driver, for the most > part, assumed it was signed. In other places, it uses `char` to mean an > unsigned number, but only in cases when the values are small. And in > still other places, `char` is used as a boolean. Put an end to this > confusion by declaring explicit types, depending on the context. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Stanislaw Gruszka <stf_xl@wp.pl> > Cc: Helmut Schaa <helmut.schaa@googlemail.com> > Cc: Kalle Valo <kvalo@kernel.org> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Acked-by: Stanislaw Gruszka <stf_xl@wp.pl> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] wifi: rt2x00: use explicitly signed or unsigned types 2022-10-20 10:29 ` Stanislaw Gruszka @ 2022-10-20 10:36 ` Kalle Valo 0 siblings, 0 replies; 20+ messages in thread From: Kalle Valo @ 2022-10-20 10:36 UTC (permalink / raw) To: Stanislaw Gruszka Cc: Jason A. Donenfeld, linux-wireless, netdev, linux-kernel, Andrew Morton, Andy Shevchenko, Helmut Schaa Stanislaw Gruszka <stf_xl@wp.pl> writes: > On Wed, Oct 19, 2022 at 09:55:41AM -0600, Jason A. Donenfeld wrote: >> On some platforms, `char` is unsigned, but this driver, for the most >> part, assumed it was signed. In other places, it uses `char` to mean an >> unsigned number, but only in cases when the values are small. And in >> still other places, `char` is used as a boolean. Put an end to this >> confusion by declaring explicit types, depending on the context. >> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> Cc: Stanislaw Gruszka <stf_xl@wp.pl> >> Cc: Helmut Schaa <helmut.schaa@googlemail.com> >> Cc: Kalle Valo <kvalo@kernel.org> >> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > > Acked-by: Stanislaw Gruszka <stf_xl@wp.pl> Thanks, I'll queue this to v6.1. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] wifi: rt2x00: use explicitly signed or unsigned types 2022-10-19 15:55 ` [PATCH v3] " Jason A. Donenfeld 2022-10-20 10:29 ` Stanislaw Gruszka @ 2022-10-21 6:48 ` Kalle Valo 2022-10-21 6:58 ` Kalle Valo 2022-10-21 7:00 ` Kalle Valo 2 siblings, 1 reply; 20+ messages in thread From: Kalle Valo @ 2022-10-21 6:48 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-wireless, netdev, linux-kernel, Jason A. Donenfeld, Andrew Morton, Andy Shevchenko, Stanislaw Gruszka, Helmut Schaa "Jason A. Donenfeld" <Jason@zx2c4.com> wrote: > On some platforms, `char` is unsigned, but this driver, for the most > part, assumed it was signed. In other places, it uses `char` to mean an > unsigned number, but only in cases when the values are small. And in > still other places, `char` is used as a boolean. Put an end to this > confusion by declaring explicit types, depending on the context. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Stanislaw Gruszka <stf_xl@wp.pl> > Cc: Helmut Schaa <helmut.schaa@googlemail.com> > Cc: Kalle Valo <kvalo@kernel.org> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > Acked-by: Stanislaw Gruszka <stf_xl@wp.pl> Patch applied to wireless.git, thanks. 3347d22eb90b wifi: rt2x00: use explicitly signed or unsigned types -- https://patchwork.kernel.org/project/linux-wireless/patch/20221019155541.3410813-1-Jason@zx2c4.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] wifi: rt2x00: use explicitly signed or unsigned types 2022-10-21 6:48 ` Kalle Valo @ 2022-10-21 6:58 ` Kalle Valo 0 siblings, 0 replies; 20+ messages in thread From: Kalle Valo @ 2022-10-21 6:58 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-wireless, netdev, linux-kernel, Andrew Morton, Andy Shevchenko, Stanislaw Gruszka, Helmut Schaa Kalle Valo <kvalo@kernel.org> writes: > "Jason A. Donenfeld" <Jason@zx2c4.com> wrote: > >> On some platforms, `char` is unsigned, but this driver, for the most >> part, assumed it was signed. In other places, it uses `char` to mean an >> unsigned number, but only in cases when the values are small. And in >> still other places, `char` is used as a boolean. Put an end to this >> confusion by declaring explicit types, depending on the context. >> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> Cc: Stanislaw Gruszka <stf_xl@wp.pl> >> Cc: Helmut Schaa <helmut.schaa@googlemail.com> >> Cc: Kalle Valo <kvalo@kernel.org> >> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> >> Acked-by: Stanislaw Gruszka <stf_xl@wp.pl> > > Patch applied to wireless.git, thanks. > > 3347d22eb90b wifi: rt2x00: use explicitly signed or unsigned types Please disregard this commit id, I used a wrong baseline so I need to apply this again. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] wifi: rt2x00: use explicitly signed or unsigned types 2022-10-19 15:55 ` [PATCH v3] " Jason A. Donenfeld 2022-10-20 10:29 ` Stanislaw Gruszka 2022-10-21 6:48 ` Kalle Valo @ 2022-10-21 7:00 ` Kalle Valo 2 siblings, 0 replies; 20+ messages in thread From: Kalle Valo @ 2022-10-21 7:00 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-wireless, netdev, linux-kernel, Jason A. Donenfeld, Andrew Morton, Andy Shevchenko, Stanislaw Gruszka, Helmut Schaa "Jason A. Donenfeld" <Jason@zx2c4.com> wrote: > On some platforms, `char` is unsigned, but this driver, for the most > part, assumed it was signed. In other places, it uses `char` to mean an > unsigned number, but only in cases when the values are small. And in > still other places, `char` is used as a boolean. Put an end to this > confusion by declaring explicit types, depending on the context. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Stanislaw Gruszka <stf_xl@wp.pl> > Cc: Helmut Schaa <helmut.schaa@googlemail.com> > Cc: Kalle Valo <kvalo@kernel.org> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > Acked-by: Stanislaw Gruszka <stf_xl@wp.pl> Patch applied to wireless.git, thanks. 66063033f77e wifi: rt2x00: use explicitly signed or unsigned types -- https://patchwork.kernel.org/project/linux-wireless/patch/20221019155541.3410813-1-Jason@zx2c4.com/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] wifi: rt2x00: use explicitly signed type for clamping 2022-10-18 20:27 ` [PATCH] wifi: rt2x00: use explicitly signed type for clamping Jason A. Donenfeld ` (2 preceding siblings ...) 2022-10-19 8:14 ` [PATCH v2] wifi: rt2x00: use explicitly signed or unsigned types Jason A. Donenfeld @ 2022-10-19 8:52 ` Stanislaw Gruszka 2022-10-19 11:04 ` Andy Shevchenko 3 siblings, 1 reply; 20+ messages in thread From: Stanislaw Gruszka @ 2022-10-19 8:52 UTC (permalink / raw) To: Jason A. Donenfeld Cc: linux-wireless, netdev, linux-kernel, Andrew Morton, Andy Shevchenko, Helmut Schaa, Kalle Valo On Tue, Oct 18, 2022 at 02:27:34PM -0600, Jason A. Donenfeld wrote: > On some platforms, `char` is unsigned, which makes casting -7 to char > overflow, which in turn makes the clamping operation bogus. Instead, > deal with an explicit `s8` type, so that the comparison is always > signed, and return an s8 result from the function as well. Note that > this function's result is assigned to a `short`, which is always signed. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Cc: Stanislaw Gruszka <stf_xl@wp.pl> > Cc: Helmut Schaa <helmut.schaa@googlemail.com> > Cc: Kalle Valo <kvalo@kernel.org> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> I prefer s8 just because is shorter name than short :-) Acked-by: Stanislaw Gruszka <stf_xl@wp.pl> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] wifi: rt2x00: use explicitly signed type for clamping 2022-10-19 8:52 ` [PATCH] wifi: rt2x00: use explicitly signed type for clamping Stanislaw Gruszka @ 2022-10-19 11:04 ` Andy Shevchenko 2022-10-20 10:40 ` Stanislaw Gruszka 0 siblings, 1 reply; 20+ messages in thread From: Andy Shevchenko @ 2022-10-19 11:04 UTC (permalink / raw) To: Stanislaw Gruszka Cc: Jason A. Donenfeld, linux-wireless, netdev, linux-kernel, Andrew Morton, Helmut Schaa, Kalle Valo On Wed, Oct 19, 2022 at 10:52:19AM +0200, Stanislaw Gruszka wrote: > On Tue, Oct 18, 2022 at 02:27:34PM -0600, Jason A. Donenfeld wrote: > > On some platforms, `char` is unsigned, which makes casting -7 to char > > overflow, which in turn makes the clamping operation bogus. Instead, > > deal with an explicit `s8` type, so that the comparison is always > > signed, and return an s8 result from the function as well. Note that > > this function's result is assigned to a `short`, which is always signed. > > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Cc: Stanislaw Gruszka <stf_xl@wp.pl> > > Cc: Helmut Schaa <helmut.schaa@googlemail.com> > > Cc: Kalle Valo <kvalo@kernel.org> > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > > I prefer s8 just because is shorter name than short :-) Shouldn't the corresponding data structure type be fixed accordingly? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] wifi: rt2x00: use explicitly signed type for clamping 2022-10-19 11:04 ` Andy Shevchenko @ 2022-10-20 10:40 ` Stanislaw Gruszka 0 siblings, 0 replies; 20+ messages in thread From: Stanislaw Gruszka @ 2022-10-20 10:40 UTC (permalink / raw) To: Andy Shevchenko Cc: Jason A. Donenfeld, linux-wireless, netdev, linux-kernel, Andrew Morton, Helmut Schaa, Kalle Valo On Wed, Oct 19, 2022 at 02:04:57PM +0300, Andy Shevchenko wrote: > On Wed, Oct 19, 2022 at 10:52:19AM +0200, Stanislaw Gruszka wrote: > > On Tue, Oct 18, 2022 at 02:27:34PM -0600, Jason A. Donenfeld wrote: > > > On some platforms, `char` is unsigned, which makes casting -7 to char > > > overflow, which in turn makes the clamping operation bogus. Instead, > > > deal with an explicit `s8` type, so that the comparison is always > > > signed, and return an s8 result from the function as well. Note that > > > this function's result is assigned to a `short`, which is always signed. > > > > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > Cc: Stanislaw Gruszka <stf_xl@wp.pl> > > > Cc: Helmut Schaa <helmut.schaa@googlemail.com> > > > Cc: Kalle Valo <kvalo@kernel.org> > > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > > > > I prefer s8 just because is shorter name than short :-) > > Shouldn't the corresponding data structure type be fixed accordingly? We can change types of channel_info default_power* fields in rt2x00.h, but I'm a bit reluctant to do so, as I'm afraid this could change actual power values sent to the hardware and will require careful verification. Regards Stanislaw ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-10-21 7:00 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <202210190108.ESC3pc3D-lkp@intel.com>
2022-10-18 20:27 ` [PATCH] wifi: rt2x00: use explicitly signed type for clamping Jason A. Donenfeld
2022-10-18 20:40 ` Andy Shevchenko
2022-10-18 20:52 ` Jason A. Donenfeld
2022-10-18 20:57 ` Andy Shevchenko
2022-10-18 20:58 ` Jason A. Donenfeld
2022-10-18 21:10 ` Andy Shevchenko
2022-10-19 8:01 ` David Laight
2022-10-18 21:07 ` Andrew Morton
2022-10-19 8:14 ` [PATCH v2] wifi: rt2x00: use explicitly signed or unsigned types Jason A. Donenfeld
2022-10-19 9:00 ` Stanislaw Gruszka
2022-10-19 15:54 ` Jason A. Donenfeld
2022-10-19 15:55 ` [PATCH v3] " Jason A. Donenfeld
2022-10-20 10:29 ` Stanislaw Gruszka
2022-10-20 10:36 ` Kalle Valo
2022-10-21 6:48 ` Kalle Valo
2022-10-21 6:58 ` Kalle Valo
2022-10-21 7:00 ` Kalle Valo
2022-10-19 8:52 ` [PATCH] wifi: rt2x00: use explicitly signed type for clamping Stanislaw Gruszka
2022-10-19 11:04 ` Andy Shevchenko
2022-10-20 10:40 ` Stanislaw Gruszka
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).