On 16/03/16 20:51, Arnd Bergmann wrote:
gcc-6 warns about passing negative signed integer into swab16()
in the dt282x driver:
drivers/staging/comedi/drivers/dt282x.c: In function 'dt282x_load_changain':
include/uapi/linux/swab.h:14:33: warning: integer overflow in expression [-Woverflow]
(((__u16)(x) & (__u16)0xff00U) >> 8)))
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
include/uapi/linux/swab.h:107:2: note: in expansion of macro '___constant_swab16'
___constant_swab16(x) : \
^~~~~~~~~~~~~~~~~~
include/uapi/linux/byteorder/big_endian.h:34:43: note: in expansion of macro '__swab16'
#define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
^~~~~~~~
include/linux/byteorder/generic.h:89:21: note: in expansion of macro '__cpu_to_le16'
#define cpu_to_le16 __cpu_to_le16
^~~~~~~~~~~~~
arch/arm/include/asm/io.h:250:6: note: in expansion of macro 'cpu_to_le16'
cpu_to_le16(v),__io(p)); })
^~~~~~~~~~~
drivers/staging/comedi/drivers/dt282x.c:566:2: note: in expansion of macro 'outw'
outw(DT2821_CHANCSR_LLE | DT2821_CHANCSR_NUMB(n),
^~~~
The warning makes sense, though the code is correct as far as I
can tell.
This disambiguates the operation by making the constant expressions
we pass here explicitly 'unsigned', which helps to avoid the warning.
As pointed out by Hartley Sweeten, scripts/checkpatch.pl notices that
the shifts here are rather unreadable, though the suggested BIT()
macro wouldn't work either. I'm changing it to a hexadecimal notation,
which hopefully improves readability. I'm leaving the DT2821_CHANCSR_PRESLA
alone because it seems wrong.
Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
---
v2: also reformat to make checkpatch.pl happy
drivers/staging/comedi/drivers/dt282x.c | 72 ++++++++++++++++-----------------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt282x.c b/drivers/staging/comedi/drivers/dt282x.c
[snip]
-#define DT2821_CHANCSR_LLE (1 << 15)
-#define DT2821_CHANCSR_PRESLA(x) (((x) & 0xf) >> 8)
-#define DT2821_CHANCSR_NUMB(x) ((((x) - 1) & 0xf) << 0)
+#define DT2821_CHANCSR_LLE 0x8000u
+#define DT2821_CHANCSR_PRESLA(x) (((x) & 0xf) >> 8) /* always 0? */
The patch is fine, thanks.
Just a note on that dodgy-looking '>>' in the DT2821_CHANCSR_PRESLA
macro.... I seems to be a typo in 0f8e8c5ab67a ("staging: comedi:
dt282x: tidy up the register map and bit defines"). It should be a
left-shift. Fortunately, it isn't used, but we ought to correct it
sometime.
Reviewed-by: Ian Abbott <abbotti@xxxxxxxxx>
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=-
-=( Web: http://www.mev.co.uk/ )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
