Re: [PATCH 4/8] staging: pi433: add parentheses to mask and shift

2017-12-13 Thread Marcus Wolf 



Am 13.12.2017 um 16:21 schrieb Valentin Vidic:

Fixes checkpatch warning:

   WARNING: Possible precedence defect with mask then right shift - may need 
parentheses

Signed-off-by: Valentin Vidic 
---
  drivers/staging/pi433/rf69.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index f77ecd60f43a..b1e243e5bcac 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -102,7 +102,7 @@ enum modulation rf69_get_modulation(struct spi_device *spi)
  
  	currentValue = rf69_read_reg(spi, REG_DATAMODUL);
  
-	switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) { // TODO improvement: change 3 to define

+   switch ((currentValue & MASK_DATAMODUL_MODULATION_TYPE) >> 3) { // TODO 
improvement: change 3 to define


As mentioned by Dan, this change isn't needed any more, since ther was a 
bug, that's already fixed.



case DATAMODUL_MODULATION_TYPE_OOK: return OOK;
case DATAMODUL_MODULATION_TYPE_FSK: return FSK;
default:return UNDEF;
@@ -340,7 +340,7 @@ enum lnaGain rf69_get_lna_gain(struct spi_device *spi)
  
  	currentValue = rf69_read_reg(spi, REG_LNA);
  
-	switch (currentValue & MASK_LNA_CURRENT_GAIN >> 3) { // improvement: change 3 to define

+   switch ((currentValue & MASK_LNA_CURRENT_GAIN) >> 3) { // improvement: 
change 3 to define


To my knowledge,'>>' is stronger than '&'.
So this change will modify the behaviour.
If I am wrong, the code was wrong from the beginning.

What should happen here, is:
read the value from reg and do a bitwise and with the shifted value from 
MASK_...



case LNA_GAIN_AUTO: return automatic;
case LNA_GAIN_MAX:  return max;
case LNA_GAIN_MAX_MINUS_6:  return maxMinus6;



--
Smarthome-Wolf UG (haftungsbeschränkt)
Helene-Lange-Weg 23
80637 München
Amtsgericht München, HRB 223529
Umastzsteuer-ID: DE304719911
Geschäftsführer: Marcus Wolf
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/8] staging: pi433: add parentheses to mask and shift

2017-12-13 Thread Dan Carpenter 
On Wed, Dec 13, 2017 at 03:21:52PM +0100, Valentin Vidic wrote:
> Fixes checkpatch warning:
> 
>   WARNING: Possible precedence defect with mask then right shift - may need 
> parentheses
> 
> Signed-off-by: Valentin Vidic 

Please do your work against linux-next or staging-next.  There have been
a ton of changes.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/8] staging: pi433: add parentheses to mask and shift

2017-12-13 Thread Valentin Vidic 
Fixes checkpatch warning:

  WARNING: Possible precedence defect with mask then right shift - may need 
parentheses

Signed-off-by: Valentin Vidic 
---
 drivers/staging/pi433/rf69.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index f77ecd60f43a..b1e243e5bcac 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -102,7 +102,7 @@ enum modulation rf69_get_modulation(struct spi_device *spi)
 
currentValue = rf69_read_reg(spi, REG_DATAMODUL);
 
-   switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) { // TODO 
improvement: change 3 to define
+   switch ((currentValue & MASK_DATAMODUL_MODULATION_TYPE) >> 3) { // TODO 
improvement: change 3 to define
case DATAMODUL_MODULATION_TYPE_OOK: return OOK;
case DATAMODUL_MODULATION_TYPE_FSK: return FSK;
default:return UNDEF;
@@ -340,7 +340,7 @@ enum lnaGain rf69_get_lna_gain(struct spi_device *spi)
 
currentValue = rf69_read_reg(spi, REG_LNA);
 
-   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;
case LNA_GAIN_MAX_MINUS_6:  return maxMinus6;
-- 
2.15.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel