Am 19.07.2017 11:51, schrieb Dan Carpenter: > We had intended to do the mask first and then the shift. Shift has > higher precedence so we need to add parenthesis. > > Fixes: 874bcba65f9a ("staging: pi433: New driver") > Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com> > > diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c > index e391ce777bc7..5089449bf775 100644 > --- a/drivers/staging/pi433/rf69.c > +++ b/drivers/staging/pi433/rf69.c > @@ -387,7 +387,7 @@ enum lnaGain rf69_get_lna_gain(struct spi_device *spi) > > currentValue = READ_REG(REG_LNA);
is currentValue used again later ? iff not (IMHO) it would be more readable to use currentValue &= MASK_LNA_CURRENT_GAIN here. and currentValue >>3 inside the switch() > > - switch (currentValue & MASK_LNA_CURRENT_GAIN >> 3) // improvement: > change 3 to define > + switch ((currentValue & MASK_LNA_CURRENT_GAIN) >> 3) // improvement: > change 3 to define > { > case LNA_GAIN_AUTO: return automatic; > case LNA_GAIN_MAX: return max; note: and instead to add an artificial define for 3, it would be simpler to change the LNA_GAIN_AUTO/MAX to the "real" values and drop the >>3 for good. just my 2 cents, re, wh > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > _______________________________________________ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel