Hello Marcus Wolf,
The patch 874bcba65f9a: "staging: pi433: New driver" from Jul 16,
2017, leads to the following static checker warning:
drivers/staging/pi433/rf69.c:104 rf69_get_modulation()
warn: shift has higher precedence than mask
drivers/staging/pi433/rf69.c:206 rf69_set_deviation()
warn: bitwise AND condition is false here
drivers/staging/pi433/rf69.c
81 int rf69_set_modulation(struct spi_device *spi, enum modulation modulation)
82 {
83 #ifdef DEBUG
84 dev_dbg(&spi->dev, "set: modulation");
85 #endif
86
87 switch (modulation) {
88 case OOK: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_TYPE) | DATAMODUL_MODULATION_TYPE_OOK);
89 case FSK: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_TYPE) | DATAMODUL_MODULATION_TYPE_FSK);
These were probably supposed to be shifted << 3:
case OOK: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_TYPE) | (DATAMODUL_MODULATION_TYPE_OOK << 3));
case FSK: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_TYPE) | (DATAMODUL_MODULATION_TYPE_FSK << 3));
90 default: INVALID_PARAM;
91 }
92 }
93
94 enum modulation rf69_get_modulation(struct spi_device *spi)
95 {
96 u8 currentValue;
97
98 #ifdef DEBUG
99 dev_dbg(&spi->dev, "get: mode");
100 #endif
101
102 currentValue = READ_REG(REG_DATAMODUL);
103
104 switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) // TODO improvement: change 3 to define
This was probably supposed to be:
switch ((currentValue & MASK_DATAMODUL_MODULATION_TYPE) >> 3)
The two bugs almost cancle each other out?
105 {
106 case DATAMODUL_MODULATION_TYPE_OOK: return OOK;
107 case DATAMODUL_MODULATION_TYPE_FSK: return FSK;
108 default: return undefined;
109 }
110 }
[ snip ]
198 // calculate register settings
199 f_reg = deviation * factor;
200 do_div(f_reg , f_step);
201
202 msb = (f_reg&0xff00) >> 8;
203 lsb = (f_reg&0xff);
204
205 // check msb
206 if (msb & !FDEVMASB_MASK)
My guess is that bitwise negate was intended here:
if (msb & ~FDEVMASB_MASK)
207 {
208 dev_dbg(&spi->dev, "set_deviation: err in calc of msb");
209 INVALID_PARAM;
210 }
211
212 // write to chip
213 retval = WRITE_REG(REG_FDEV_MSB, msb);
214 if (retval) return retval;
215 retval = WRITE_REG(REG_FDEV_LSB, lsb);
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
