On Wednesday, March 16, 2016 1:51 PM, 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), > ^~~~ Arnd, Is this a gcc-6 specific issue? Seems line this warning should be showing up in a lot of drivers. > 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. BIT() should work for the ones pointed out by checpatch.pl. I would argue that the hexadecimal notation is still rather unreadable. These ones make my head hurt... -#define DT2821_ADCSR_GS(x) (((x) & 0x3) << 4) +#define DT2821_ADCSR_GS(x) (0x0030u & ((x) << 4)) -#define DT2821_DACSR_YSEL(x) ((x) << 9) +#define DT2821_DACSR_YSEL(x) (0x7e00u & (x) << 9) -#define DT2821_SUPCSR_DS_PIO (0 << 10) -#define DT2821_SUPCSR_DS_AD_CLK (1 << 10) -#define DT2821_SUPCSR_DS_DA_CLK (2 << 10) -#define DT2821_SUPCSR_DS_AD_TRIG (3 << 10) +#define DT2821_SUPCSR_DS_PIO (0x0c00u & (0u << 10)) +#define DT2821_SUPCSR_DS_AD_CLK (0x0c00u & (1u << 10)) +#define DT2821_SUPCSR_DS_DA_CLK (0x0c00u & (2u << 10)) +#define DT2821_SUPCSR_DS_AD_TRIG (0x0c00u & (3u << 10)) Also, most of the comedi drivers use the BIT() macro. Are you planning on changing all of them to use hexadecimal notation? Regards, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
