[PATCH 00/11] treewide: mask then shift defects and style updates

[PATCH 00/11] treewide: mask then shift defects and style updates

[Joe Perches]

logical mask has lower precedence than shift but should be
done before the shift so parentheses are generally required.

And when masking with a fixed value after a shift, normal kernel
style has the shift on the left, then the shift on the right so
convert a few non-conforming uses.

Joe Perches (11):
  block: nvme-scsi: Fix probable mask then right shift defects
  radeon: evergreen: Fix probable mask then right shift defects
  aiptek: Fix probable mask then right shift defects
  dvb-net: Fix probable mask then right shift defects
  cx25840/cx18: Use standard ordering of mask and shift
  wm8350-core: Fix probable mask then right shift defect
  iwlwifi: dvm: Fix probable mask then right shift defect
  ssb: driver_chip_comon_pmu: Fix probable mask then right shift defect
  tty: ipwireless: Fix probable mask then right shift defects
  hwa-hc: Fix probable mask then right shift defect
  sound: ad1889: Fix probable mask then right shift defects

 drivers/block/nvme-scsi.c                | 12 ++++++------
 drivers/gpu/drm/radeon/evergreen.c       |  3 ++-
 drivers/input/tablet/aiptek.c            |  6 +++---
 drivers/media/dvb-core/dvb_net.c         |  4 +++-
 drivers/media/i2c/cx25840/cx25840-core.c | 12 ++++++------
 drivers/media/pci/cx18/cx18-av-core.c    | 16 ++++++++--------
 drivers/mfd/wm8350-core.c                |  2 +-
 drivers/net/wireless/iwlwifi/dvm/lib.c   |  4 ++--
 drivers/ssb/driver_chipcommon_pmu.c      |  4 ++--
 drivers/tty/ipwireless/hardware.c        | 12 ++++++------
 drivers/usb/host/hwa-hc.c                |  2 +-
 sound/pci/ad1889.c                       |  8 ++++----
 12 files changed, 44 insertions(+), 41 deletions(-)

-- 
2.1.2

[PATCH 01/11] block: nvme-scsi: Fix probable mask then right shift defects

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the mask.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/block/nvme-scsi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/block/nvme-scsi.c b/drivers/block/nvme-scsi.c
index a4cd6d6..529ee54 100644
--- a/drivers/block/nvme-scsi.c
+++ b/drivers/block/nvme-scsi.c
@@ -1972,8 +1972,8 @@ static inline void nvme_trans_get_io_cdb10(u8 *cmd,
 {
 	cdb_info->fua = GET_U8_FROM_CDB(cmd, IO_10_CDB_FUA_OFFSET) &
 					IO_CDB_FUA_MASK;
-	cdb_info->prot_info = GET_U8_FROM_CDB(cmd, IO_10_CDB_WP_OFFSET) &
-					IO_CDB_WP_MASK >> IO_CDB_WP_SHIFT;
+	cdb_info->prot_info = (GET_U8_FROM_CDB(cmd, IO_10_CDB_WP_OFFSET) &
+			       IO_CDB_WP_MASK) >> IO_CDB_WP_SHIFT;
 	cdb_info->lba = GET_U32_FROM_CDB(cmd, IO_10_CDB_LBA_OFFSET);
 	cdb_info->xfer_len = GET_U16_FROM_CDB(cmd, IO_10_CDB_TX_LEN_OFFSET);
 }
@@ -1983,8 +1983,8 @@ static inline void nvme_trans_get_io_cdb12(u8 *cmd,
 {
 	cdb_info->fua = GET_U8_FROM_CDB(cmd, IO_12_CDB_FUA_OFFSET) &
 					IO_CDB_FUA_MASK;
-	cdb_info->prot_info = GET_U8_FROM_CDB(cmd, IO_12_CDB_WP_OFFSET) &
-					IO_CDB_WP_MASK >> IO_CDB_WP_SHIFT;
+	cdb_info->prot_info = (GET_U8_FROM_CDB(cmd, IO_12_CDB_WP_OFFSET) &
+			       IO_CDB_WP_MASK) >> IO_CDB_WP_SHIFT;
 	cdb_info->lba = GET_U32_FROM_CDB(cmd, IO_12_CDB_LBA_OFFSET);
 	cdb_info->xfer_len = GET_U32_FROM_CDB(cmd, IO_12_CDB_TX_LEN_OFFSET);
 }
@@ -1994,8 +1994,8 @@ static inline void nvme_trans_get_io_cdb16(u8 *cmd,
 {
 	cdb_info->fua = GET_U8_FROM_CDB(cmd, IO_16_CDB_FUA_OFFSET) &
 					IO_CDB_FUA_MASK;
-	cdb_info->prot_info = GET_U8_FROM_CDB(cmd, IO_16_CDB_WP_OFFSET) &
-					IO_CDB_WP_MASK >> IO_CDB_WP_SHIFT;
+	cdb_info->prot_info = (GET_U8_FROM_CDB(cmd, IO_16_CDB_WP_OFFSET) &
+			       IO_CDB_WP_MASK) >> IO_CDB_WP_SHIFT;
 	cdb_info->lba = GET_U64_FROM_CDB(cmd, IO_16_CDB_LBA_OFFSET);
 	cdb_info->xfer_len = GET_U32_FROM_CDB(cmd, IO_16_CDB_TX_LEN_OFFSET);
 }
-- 
2.1.2

Re: [PATCH 01/11] block: nvme-scsi: Fix probable mask then right shift defects

[Verma, Vishal L]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2410 bytes --]

On Sun, 2014-10-26 at 22:24 -0700, Joe Perches wrote:
> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
> 
> Add parentheses around the mask.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Acked-by: Vishal Verma <vishal.l.verma@linux.intel.com>

> ---
>  drivers/block/nvme-scsi.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/block/nvme-scsi.c b/drivers/block/nvme-scsi.c
> index a4cd6d6..529ee54 100644
> --- a/drivers/block/nvme-scsi.c
> +++ b/drivers/block/nvme-scsi.c
> @@ -1972,8 +1972,8 @@ static inline void nvme_trans_get_io_cdb10(u8 *cmd,
>  {
>  	cdb_info->fua = GET_U8_FROM_CDB(cmd, IO_10_CDB_FUA_OFFSET) &
>  					IO_CDB_FUA_MASK;
> -	cdb_info->prot_info = GET_U8_FROM_CDB(cmd, IO_10_CDB_WP_OFFSET) &
> -					IO_CDB_WP_MASK >> IO_CDB_WP_SHIFT;
> +	cdb_info->prot_info = (GET_U8_FROM_CDB(cmd, IO_10_CDB_WP_OFFSET) &
> +			       IO_CDB_WP_MASK) >> IO_CDB_WP_SHIFT;
>  	cdb_info->lba = GET_U32_FROM_CDB(cmd, IO_10_CDB_LBA_OFFSET);
>  	cdb_info->xfer_len = GET_U16_FROM_CDB(cmd, IO_10_CDB_TX_LEN_OFFSET);
>  }
> @@ -1983,8 +1983,8 @@ static inline void nvme_trans_get_io_cdb12(u8 *cmd,
>  {
>  	cdb_info->fua = GET_U8_FROM_CDB(cmd, IO_12_CDB_FUA_OFFSET) &
>  					IO_CDB_FUA_MASK;
> -	cdb_info->prot_info = GET_U8_FROM_CDB(cmd, IO_12_CDB_WP_OFFSET) &
> -					IO_CDB_WP_MASK >> IO_CDB_WP_SHIFT;
> +	cdb_info->prot_info = (GET_U8_FROM_CDB(cmd, IO_12_CDB_WP_OFFSET) &
> +			       IO_CDB_WP_MASK) >> IO_CDB_WP_SHIFT;
>  	cdb_info->lba = GET_U32_FROM_CDB(cmd, IO_12_CDB_LBA_OFFSET);
>  	cdb_info->xfer_len = GET_U32_FROM_CDB(cmd, IO_12_CDB_TX_LEN_OFFSET);
>  }
> @@ -1994,8 +1994,8 @@ static inline void nvme_trans_get_io_cdb16(u8 *cmd,
>  {
>  	cdb_info->fua = GET_U8_FROM_CDB(cmd, IO_16_CDB_FUA_OFFSET) &
>  					IO_CDB_FUA_MASK;
> -	cdb_info->prot_info = GET_U8_FROM_CDB(cmd, IO_16_CDB_WP_OFFSET) &
> -					IO_CDB_WP_MASK >> IO_CDB_WP_SHIFT;
> +	cdb_info->prot_info = (GET_U8_FROM_CDB(cmd, IO_16_CDB_WP_OFFSET) &
> +			       IO_CDB_WP_MASK) >> IO_CDB_WP_SHIFT;
>  	cdb_info->lba = GET_U64_FROM_CDB(cmd, IO_16_CDB_LBA_OFFSET);
>  	cdb_info->xfer_len = GET_U32_FROM_CDB(cmd, IO_16_CDB_TX_LEN_OFFSET);
>  }

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH 02/11] radeon: evergreen: Fix probable mask then right shift defects

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the mask.

Use the already #defined values instead of hardcoding.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/gpu/drm/radeon/evergreen.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index a31f1ca..a97a685 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -3303,7 +3303,8 @@ static void evergreen_gpu_init(struct radeon_device *rdev)
 	rdev->config.evergreen.tile_config |=
 		((gb_addr_config & 0x30000000) >> 28) << 12;
 
-	num_shader_engines = (gb_addr_config & NUM_SHADER_ENGINES(3) >> 12) + 1;
+	num_shader_engines = ((gb_addr_config & NUM_SHADER_ENGINES_MASK)
+			      >> NUM_SHADER_ENGINES) + 1;
 
 	if ((rdev->family >= CHIP_CEDAR) && (rdev->family <= CHIP_HEMLOCK)) {
 		u32 efuse_straps_4;
-- 
2.1.2

Re: [PATCH 02/11] radeon: evergreen: Fix probable mask then right shift defects

[Michel Dänzer]

On 27.10.2014 14:24, Joe Perches wrote:
> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
>
> Add parentheses around the mask.
>
> Use the already #defined values instead of hardcoding.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>   drivers/gpu/drm/radeon/evergreen.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
> index a31f1ca..a97a685 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -3303,7 +3303,8 @@ static void evergreen_gpu_init(struct radeon_device *rdev)
>   	rdev->config.evergreen.tile_config |=
>   		((gb_addr_config & 0x30000000) >> 28) << 12;
>
> -	num_shader_engines = (gb_addr_config & NUM_SHADER_ENGINES(3) >> 12) + 1;
> +	num_shader_engines = ((gb_addr_config & NUM_SHADER_ENGINES_MASK)
> +			      >> NUM_SHADER_ENGINES) + 1;
                                  ^^^^^^^^^^^^^^^^^^
I think this should be NUM_SHADER_ENGINES_SHIFT?

Looks good to me other than that.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

[PATCH 02/11 V2] radeon: evergreen: Fix probable mask then right shift defect

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the mask.

Use the already #defined values instead of hardcoding.

Signed-off-by: Joe Perches <joe@perches.com>
---
> I think this should be NUM_SHADER_ENGINES_SHIFT?

(Joe can't type)

exactly right, thanks Michel

 drivers/gpu/drm/radeon/evergreen.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index a31f1ca..a97a685 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -3303,7 +3303,8 @@ static void evergreen_gpu_init(struct radeon_device *rdev)
 	rdev->config.evergreen.tile_config |=
 		((gb_addr_config & 0x30000000) >> 28) << 12;
 
-	num_shader_engines = (gb_addr_config & NUM_SHADER_ENGINES(3) >> 12) + 1;
+	num_shader_engines = ((gb_addr_config & NUM_SHADER_ENGINES_MASK)
+			      >> NUM_SHADER_ENGINES_SHIFT) + 1;
 
 	if ((rdev->family >= CHIP_CEDAR) && (rdev->family <= CHIP_HEMLOCK)) {
 		u32 efuse_straps_4;

Re: [PATCH 02/11 V2] radeon: evergreen: Fix probable mask then right shift defect

[Michel Dänzer]

On 27.10.2014 23:14, Joe Perches wrote:
> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
>
> Add parentheses around the mask.
>
> Use the already #defined values instead of hardcoding.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>> I think this should be NUM_SHADER_ENGINES_SHIFT?
>
> (Joe can't type)
>
> exactly right, thanks Michel
>
>   drivers/gpu/drm/radeon/evergreen.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
> index a31f1ca..a97a685 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -3303,7 +3303,8 @@ static void evergreen_gpu_init(struct radeon_device *rdev)
>   	rdev->config.evergreen.tile_config |=
>   		((gb_addr_config & 0x30000000) >> 28) << 12;
>
> -	num_shader_engines = (gb_addr_config & NUM_SHADER_ENGINES(3) >> 12) + 1;
> +	num_shader_engines = ((gb_addr_config & NUM_SHADER_ENGINES_MASK)
> +			      >> NUM_SHADER_ENGINES_SHIFT) + 1;
>
>   	if ((rdev->family >= CHIP_CEDAR) && (rdev->family <= CHIP_HEMLOCK)) {
>   		u32 efuse_straps_4;

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

Re: [PATCH 02/11 V2] radeon: evergreen: Fix probable mask then right shift defect

[Alex Deucher]

On Mon, Oct 27, 2014 at 10:14 AM, Joe Perches <joe@perches.com> wrote:
> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
>
> Add parentheses around the mask.
>
> Use the already #defined values instead of hardcoding.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>> I think this should be NUM_SHADER_ENGINES_SHIFT?
>
> (Joe can't type)
>
> exactly right, thanks Michel

Applied with a compile fix.

Thanks,

Alex

>
>  drivers/gpu/drm/radeon/evergreen.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
> index a31f1ca..a97a685 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -3303,7 +3303,8 @@ static void evergreen_gpu_init(struct radeon_device *rdev)
>         rdev->config.evergreen.tile_config |=
>                 ((gb_addr_config & 0x30000000) >> 28) << 12;
>
> -       num_shader_engines = (gb_addr_config & NUM_SHADER_ENGINES(3) >> 12) + 1;
> +       num_shader_engines = ((gb_addr_config & NUM_SHADER_ENGINES_MASK)
> +                             >> NUM_SHADER_ENGINES_SHIFT) + 1;
>
>         if ((rdev->family >= CHIP_CEDAR) && (rdev->family <= CHIP_HEMLOCK)) {
>                 u32 efuse_straps_4;
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/11 V2] radeon: evergreen: Fix probable mask then right shift defect

[Michel Dänzer]

On 28.10.2014 23:06, Alex Deucher wrote:
> On Mon, Oct 27, 2014 at 10:14 AM, Joe Perches <joe@perches.com> wrote:
>> Precedence of & and >> is not the same and is not left to right.
>> shift has higher precedence and should be done after the mask.
>>
>> Add parentheses around the mask.
>>
>> Use the already #defined values instead of hardcoding.
>>
>> Signed-off-by: Joe Perches <joe@perches.com>
>> ---
>>> I think this should be NUM_SHADER_ENGINES_SHIFT?
>>
>> (Joe can't type)
>>
>> exactly right, thanks Michel
>
> Applied with a compile fix.

Joe, in the future please make sure your patches compile before 
submitting them.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer

[PATCH 03/11] aiptek: Fix probable mask then right shift defects

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Here the shifts are unnecessary and a non-zero value can be used
as the test to set 1 or zero.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/input/tablet/aiptek.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
index e7f966d..dee2bb9 100644
--- a/drivers/input/tablet/aiptek.c
+++ b/drivers/input/tablet/aiptek.c
@@ -489,9 +489,9 @@ static void aiptek_irq(struct urb *urb)
 			 */
 			jitterable = data[1] & 0x07;
 
-			left = (data[1] & aiptek->curSetting.mouseButtonLeft >> 2) != 0 ? 1 : 0;
-			right = (data[1] & aiptek->curSetting.mouseButtonRight >> 2) != 0 ? 1 : 0;
-			middle = (data[1] & aiptek->curSetting.mouseButtonMiddle >> 2) != 0 ? 1 : 0;
+			left = data[1] & aiptek->curSetting.mouseButtonLeft ? 1 : 0;
+			right = data[1] & aiptek->curSetting.mouseButtonRight ? 1 : 0;
+			middle = data[1] & aiptek->curSetting.mouseButtonMiddle ? 1 : 0;
 
 			input_report_key(inputdev, BTN_LEFT, left);
 			input_report_key(inputdev, BTN_MIDDLE, middle);
-- 
2.1.2

Re: [PATCH 03/11] aiptek: Fix probable mask then right shift defects

[Dmitry Torokhov]

Hi Joe,

On Sun, Oct 26, 2014 at 10:24:59PM -0700, Joe Perches wrote:
> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.

Looking at the protocol description the current code is exactly right.
We want to "move" button bits first as in packet type 1 they are in a
different place than in other packets.

I'll take a patch that adds parenthesis around shifts to make clear it
is intended.

Thanks.

> 
> Here the shifts are unnecessary and a non-zero value can be used
> as the test to set 1 or zero.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/input/tablet/aiptek.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
> index e7f966d..dee2bb9 100644
> --- a/drivers/input/tablet/aiptek.c
> +++ b/drivers/input/tablet/aiptek.c
> @@ -489,9 +489,9 @@ static void aiptek_irq(struct urb *urb)
>  			 */
>  			jitterable = data[1] & 0x07;
>  
> -			left = (data[1] & aiptek->curSetting.mouseButtonLeft >> 2) != 0 ? 1 : 0;
> -			right = (data[1] & aiptek->curSetting.mouseButtonRight >> 2) != 0 ? 1 : 0;
> -			middle = (data[1] & aiptek->curSetting.mouseButtonMiddle >> 2) != 0 ? 1 : 0;
> +			left = data[1] & aiptek->curSetting.mouseButtonLeft ? 1 : 0;
> +			right = data[1] & aiptek->curSetting.mouseButtonRight ? 1 : 0;
> +			middle = data[1] & aiptek->curSetting.mouseButtonMiddle ? 1 : 0;
>  
>  			input_report_key(inputdev, BTN_LEFT, left);
>  			input_report_key(inputdev, BTN_MIDDLE, middle);
> -- 
> 2.1.2
> 

-- 
Dmitry

Re: [PATCH 03/11] aiptek: Fix probable mask then right shift defects

[Joe Perches]

On Mon, 2014-10-27 at 07:44 -0700, Dmitry Torokhov wrote:
> Hi Joe,

Hello Dmitry.

> On Sun, Oct 26, 2014 at 10:24:59PM -0700, Joe Perches wrote:
> > Precedence of & and >> is not the same and is not left to right.
> > shift has higher precedence and should be done after the mask.
> 
> Looking at the protocol description the current code is exactly right.
> We want to "move" button bits first as in packet type 1 they are in a
> different place than in other packets.
> 
> I'll take a patch that adds parenthesis around shifts to make clear it
> is intended.

I think it's more sensible to do the shift first to a
temporary then direct comparisons.

Perhaps something like this cleanup that also uses
a temporary for aiptek->curSetting and
!!(foo & mask) instead of ((foo & mask) != 0) ? 1 : 0;
---
 drivers/input/tablet/aiptek.c | 149 ++++++++++++++++++++----------------------
 1 file changed, 72 insertions(+), 77 deletions(-)

diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
index e7f966d..9c46618 100644
--- a/drivers/input/tablet/aiptek.c
+++ b/drivers/input/tablet/aiptek.c
@@ -433,6 +433,7 @@ static const char *map_val_to_str(const struct aiptek_map *map, int val)
 static void aiptek_irq(struct urb *urb)
 {
 	struct aiptek *aiptek = urb->context;
+	struct aiptek_settings *settings = &aiptek->curSetting;
 	unsigned char *data = aiptek->data;
 	struct input_dev *inputdev = aiptek->inputdev;
 	struct usb_interface *intf = aiptek->intf;
@@ -472,26 +473,31 @@ static void aiptek_irq(struct urb *urb)
 	 * tool generated the event.
 	 */
 	if (data[0] == 1) {
-		if (aiptek->curSetting.coordinateMode ==
+		if (settings->coordinateMode ==
 		    AIPTEK_COORDINATE_ABSOLUTE_MODE) {
 			aiptek->diagnostic =
 			    AIPTEK_DIAGNOSTIC_SENDING_RELATIVE_IN_ABSOLUTE;
 		} else {
-			x = (signed char) data[2];
-			y = (signed char) data[3];
-
-			/* jitterable keeps track of whether any button has been pressed.
-			 * We're also using it to remap the physical mouse button mask
-			 * to pseudo-settings. (We don't specifically care about it's
-			 * value after moving/transposing mouse button bitmasks, except
+			/*
+			 * Shift buttons pressed to the curSettings location.
+			 * jitterable keeps track of whether any button has
+			 * been pressed.  We're also using it to remap the
+			 * physical mouse button mask to pseudo-settings.
+			 * (We don't specifically care about it's value after
+			 * moving/transposing mouse button bitmasks, except
 			 * that a non-zero value indicates that one or more
 			 * mouse button was pressed.)
 			 */
+			unsigned char buttons = data[1] << 2;
+
+			x = (signed char)data[2];
+			y = (signed char)data[3];
+
 			jitterable = data[1] & 0x07;
 
-			left = (data[1] & aiptek->curSetting.mouseButtonLeft >> 2) != 0 ? 1 : 0;
-			right = (data[1] & aiptek->curSetting.mouseButtonRight >> 2) != 0 ? 1 : 0;
-			middle = (data[1] & aiptek->curSetting.mouseButtonMiddle >> 2) != 0 ? 1 : 0;
+			left = !!(buttons & settings->mouseButtonLeft);
+			right = !!(buttons & settings->mouseButtonRight);
+			middle = !!(buttons & settings->mouseButtonMiddle);
 
 			input_report_key(inputdev, BTN_LEFT, left);
 			input_report_key(inputdev, BTN_MIDDLE, middle);
@@ -505,10 +511,10 @@ static void aiptek_irq(struct urb *urb)
 			/* Wheel support is in the form of a single-event
 			 * firing.
 			 */
-			if (aiptek->curSetting.wheel != AIPTEK_WHEEL_DISABLE) {
+			if (settings->wheel != AIPTEK_WHEEL_DISABLE) {
 				input_report_rel(inputdev, REL_WHEEL,
-						 aiptek->curSetting.wheel);
-				aiptek->curSetting.wheel = AIPTEK_WHEEL_DISABLE;
+						 settings->wheel);
+				settings->wheel = AIPTEK_WHEEL_DISABLE;
 			}
 			if (aiptek->lastMacro != -1) {
 			        input_report_key(inputdev,
@@ -522,26 +528,26 @@ static void aiptek_irq(struct urb *urb)
 	 * absolute coordinates.
 	 */
 	else if (data[0] == 2) {
-		if (aiptek->curSetting.coordinateMode == AIPTEK_COORDINATE_RELATIVE_MODE) {
+		if (settings->coordinateMode == AIPTEK_COORDINATE_RELATIVE_MODE) {
 			aiptek->diagnostic = AIPTEK_DIAGNOSTIC_SENDING_ABSOLUTE_IN_RELATIVE;
 		} else if (!AIPTEK_POINTER_ALLOW_STYLUS_MODE
-			    (aiptek->curSetting.pointerMode)) {
+			    (settings->pointerMode)) {
 				aiptek->diagnostic = AIPTEK_DIAGNOSTIC_TOOL_DISALLOWED;
 		} else {
 			x = get_unaligned_le16(data + 1);
 			y = get_unaligned_le16(data + 3);
 			z = get_unaligned_le16(data + 6);
 
-			dv = (data[5] & 0x01) != 0 ? 1 : 0;
-			p = (data[5] & 0x02) != 0 ? 1 : 0;
-			tip = (data[5] & 0x04) != 0 ? 1 : 0;
+			dv = !!(data[5] & 0x01);
+			p = !!(data[5] & 0x02);
+			tip = !!(data[5] & 0x04);
 
 			/* Use jitterable to re-arrange button masks
 			 */
 			jitterable = data[5] & 0x18;
 
-			bs = (data[5] & aiptek->curSetting.stylusButtonLower) != 0 ? 1 : 0;
-			pck = (data[5] & aiptek->curSetting.stylusButtonUpper) != 0 ? 1 : 0;
+			bs = !!(data[5] & settings->stylusButtonLower);
+			pck = !!(data[5] & settings->stylusButtonUpper);
 
 			/* dv indicates 'data valid' (e.g., the tablet is in sync
 			 * and has delivered a "correct" report) We will ignore
@@ -552,14 +558,14 @@ static void aiptek_irq(struct urb *urb)
 				 * tool key, and set the new one.
 				 */
 				if (aiptek->previousToolMode !=
-				    aiptek->curSetting.toolMode) {
+				    settings->toolMode) {
 				        input_report_key(inputdev,
 							 aiptek->previousToolMode, 0);
 					input_report_key(inputdev,
-							 aiptek->curSetting.toolMode,
+							 settings->toolMode,
 							 1);
 					aiptek->previousToolMode =
-					          aiptek->curSetting.toolMode;
+					          settings->toolMode;
 				}
 
 				if (p != 0) {
@@ -571,27 +577,25 @@ static void aiptek_irq(struct urb *urb)
 					input_report_key(inputdev, BTN_STYLUS, bs);
 					input_report_key(inputdev, BTN_STYLUS2, pck);
 
-					if (aiptek->curSetting.xTilt !=
-					    AIPTEK_TILT_DISABLE) {
+					if (settings->xTilt != AIPTEK_TILT_DISABLE) {
 						input_report_abs(inputdev,
 								 ABS_TILT_X,
-								 aiptek->curSetting.xTilt);
+								 settings->xTilt);
 					}
-					if (aiptek->curSetting.yTilt != AIPTEK_TILT_DISABLE) {
+					if (settings->yTilt != AIPTEK_TILT_DISABLE) {
 						input_report_abs(inputdev,
 								 ABS_TILT_Y,
-								 aiptek->curSetting.yTilt);
+								 settings->yTilt);
 					}
 
 					/* Wheel support is in the form of a single-event
 					 * firing.
 					 */
-					if (aiptek->curSetting.wheel !=
-					    AIPTEK_WHEEL_DISABLE) {
+					if (settings->wheel != AIPTEK_WHEEL_DISABLE) {
 						input_report_abs(inputdev,
 								 ABS_WHEEL,
-								 aiptek->curSetting.wheel);
-						aiptek->curSetting.wheel = AIPTEK_WHEEL_DISABLE;
+								 settings->wheel);
+						settings->wheel = AIPTEK_WHEEL_DISABLE;
 					}
 				}
 				input_report_abs(inputdev, ABS_MISC, p | AIPTEK_REPORT_TOOL_STYLUS);
@@ -607,10 +611,10 @@ static void aiptek_irq(struct urb *urb)
 	/* Report 3's come from the mouse in absolute mode.
 	 */
 	else if (data[0] == 3) {
-		if (aiptek->curSetting.coordinateMode == AIPTEK_COORDINATE_RELATIVE_MODE) {
+		if (settings->coordinateMode == AIPTEK_COORDINATE_RELATIVE_MODE) {
 			aiptek->diagnostic = AIPTEK_DIAGNOSTIC_SENDING_ABSOLUTE_IN_RELATIVE;
 		} else if (!AIPTEK_POINTER_ALLOW_MOUSE_MODE
-			(aiptek->curSetting.pointerMode)) {
+			(settings->pointerMode)) {
 			aiptek->diagnostic = AIPTEK_DIAGNOSTIC_TOOL_DISALLOWED;
 		} else {
 			x = get_unaligned_le16(data + 1);
@@ -618,25 +622,25 @@ static void aiptek_irq(struct urb *urb)
 
 			jitterable = data[5] & 0x1c;
 
-			dv = (data[5] & 0x01) != 0 ? 1 : 0;
-			p = (data[5] & 0x02) != 0 ? 1 : 0;
-			left = (data[5] & aiptek->curSetting.mouseButtonLeft) != 0 ? 1 : 0;
-			right = (data[5] & aiptek->curSetting.mouseButtonRight) != 0 ? 1 : 0;
-			middle = (data[5] & aiptek->curSetting.mouseButtonMiddle) != 0 ? 1 : 0;
+			dv = !!(data[5] & 0x01);
+			p = !!(data[5] & 0x02);
+			left = !!(data[5] & settings->mouseButtonLeft);
+			right = !!(data[5] & settings->mouseButtonRight);
+			middle = !!(data[5] & settings->mouseButtonMiddle);
 
 			if (dv != 0) {
 				/* If the selected tool changed, reset the old
 				 * tool key, and set the new one.
 				 */
 				if (aiptek->previousToolMode !=
-				    aiptek->curSetting.toolMode) {
+				    settings->toolMode) {
 				        input_report_key(inputdev,
 							 aiptek->previousToolMode, 0);
 					input_report_key(inputdev,
-							 aiptek->curSetting.toolMode,
+							 settings->toolMode,
 							 1);
 					aiptek->previousToolMode =
-					          aiptek->curSetting.toolMode;
+					          settings->toolMode;
 				}
 
 				if (p != 0) {
@@ -650,11 +654,11 @@ static void aiptek_irq(struct urb *urb)
 					/* Wheel support is in the form of a single-event
 					 * firing.
 					 */
-					if (aiptek->curSetting.wheel != AIPTEK_WHEEL_DISABLE) {
+					if (settings->wheel != AIPTEK_WHEEL_DISABLE) {
 						input_report_abs(inputdev,
 								 ABS_WHEEL,
-								 aiptek->curSetting.wheel);
-						aiptek->curSetting.wheel = AIPTEK_WHEEL_DISABLE;
+								 settings->wheel);
+						settings->wheel = AIPTEK_WHEEL_DISABLE;
 					}
 				}
 				input_report_abs(inputdev, ABS_MISC, p | AIPTEK_REPORT_TOOL_MOUSE);
@@ -672,11 +676,11 @@ static void aiptek_irq(struct urb *urb)
 	else if (data[0] == 4) {
 		jitterable = data[1] & 0x18;
 
-		dv = (data[1] & 0x01) != 0 ? 1 : 0;
-		p = (data[1] & 0x02) != 0 ? 1 : 0;
-		tip = (data[1] & 0x04) != 0 ? 1 : 0;
-		bs = (data[1] & aiptek->curSetting.stylusButtonLower) != 0 ? 1 : 0;
-		pck = (data[1] & aiptek->curSetting.stylusButtonUpper) != 0 ? 1 : 0;
+		dv = !!(data[1] & 0x01);
+		p = !!(data[1] & 0x02);
+		tip = !!(data[1] & 0x04);
+		bs = !!(data[1] & settings->stylusButtonLower);
+		pck = !!(data[1] & settings->stylusButtonUpper);
 
 		macro = dv && p && tip && !(data[3] & 1) ? (data[3] >> 1) : -1;
 		z = get_unaligned_le16(data + 4);
@@ -685,15 +689,12 @@ static void aiptek_irq(struct urb *urb)
 		        /* If the selected tool changed, reset the old
 			 * tool key, and set the new one.
 			 */
-		        if (aiptek->previousToolMode !=
-			    aiptek->curSetting.toolMode) {
+		        if (aiptek->previousToolMode != settings->toolMode) {
 			        input_report_key(inputdev,
 						 aiptek->previousToolMode, 0);
 				input_report_key(inputdev,
-						 aiptek->curSetting.toolMode,
-						 1);
-				aiptek->previousToolMode =
-				        aiptek->curSetting.toolMode;
+						 settings->toolMode, 1);
+				aiptek->previousToolMode = settings->toolMode;
 			}
 		}
 
@@ -715,24 +716,23 @@ static void aiptek_irq(struct urb *urb)
 	else if (data[0] == 5) {
 		jitterable = data[1] & 0x1c;
 
-		dv = (data[1] & 0x01) != 0 ? 1 : 0;
-		p = (data[1] & 0x02) != 0 ? 1 : 0;
-		left = (data[1]& aiptek->curSetting.mouseButtonLeft) != 0 ? 1 : 0;
-		right = (data[1] & aiptek->curSetting.mouseButtonRight) != 0 ? 1 : 0;
-		middle = (data[1] & aiptek->curSetting.mouseButtonMiddle) != 0 ? 1 : 0;
+		dv = !!(data[1] & 0x01);
+		p = !!(data[1] & 0x02);
+		left = !!(data[1]& settings->mouseButtonLeft);
+		right = !!(data[1] & settings->mouseButtonRight);
+		middle = !!(data[1] & settings->mouseButtonMiddle);
 		macro = dv && p && left && !(data[3] & 1) ? (data[3] >> 1) : 0;
 
 		if (dv) {
 		        /* If the selected tool changed, reset the old
 			 * tool key, and set the new one.
 			 */
-		        if (aiptek->previousToolMode !=
-			    aiptek->curSetting.toolMode) {
+		        if (aiptek->previousToolMode != settings->toolMode) {
 		                input_report_key(inputdev,
 						 aiptek->previousToolMode, 0);
 			        input_report_key(inputdev,
-						 aiptek->curSetting.toolMode, 1);
-			        aiptek->previousToolMode = aiptek->curSetting.toolMode;
+						 settings->toolMode, 1);
+			        aiptek->previousToolMode = settings->toolMode;
 			}
 		}
 
@@ -770,15 +770,10 @@ static void aiptek_irq(struct urb *urb)
 		/* If the selected tool changed, reset the old
 		   tool key, and set the new one.
 		*/
-		if (aiptek->previousToolMode !=
-		    aiptek->curSetting.toolMode) {
-		        input_report_key(inputdev,
-					 aiptek->previousToolMode, 0);
-			input_report_key(inputdev,
-					 aiptek->curSetting.toolMode,
-					 1);
-			aiptek->previousToolMode =
-				aiptek->curSetting.toolMode;
+		if (aiptek->previousToolMode != settings->toolMode) {
+		        input_report_key(inputdev, aiptek->previousToolMode, 0);
+			input_report_key(inputdev, settings->toolMode, 1);
+			aiptek->previousToolMode = settings->toolMode;
 		}
 
 		input_report_key(inputdev, macroKeyEvents[macro], 1);
@@ -802,9 +797,9 @@ static void aiptek_irq(struct urb *urb)
 	 */
 
 	if (aiptek->previousJitterable != jitterable &&
-	    aiptek->curSetting.jitterDelay != 0 && aiptek->inDelay != 1) {
+	    settings->jitterDelay != 0 && aiptek->inDelay != 1) {
 		aiptek->endDelay = jiffies +
-		    ((aiptek->curSetting.jitterDelay * HZ) / 1000);
+			((settings->jitterDelay * HZ) / 1000);
 		aiptek->inDelay = 1;
 	}
 	aiptek->previousJitterable = jitterable;

Re: [PATCH 03/11] aiptek: Fix probable mask then right shift defects

[Dmitry Torokhov]

On Monday, October 27, 2014 10:56:54 AM Joe Perches wrote:
> On Mon, 2014-10-27 at 07:44 -0700, Dmitry Torokhov wrote:
> > Hi Joe,
> 
> Hello Dmitry.
> 
> > On Sun, Oct 26, 2014 at 10:24:59PM -0700, Joe Perches wrote:
> > > Precedence of & and >> is not the same and is not left to right.
> > > shift has higher precedence and should be done after the mask.
> > 
> > Looking at the protocol description the current code is exactly right.
> > We want to "move" button bits first as in packet type 1 they are in a
> > different place than in other packets.
> > 
> > I'll take a patch that adds parenthesis around shifts to make clear it
> > is intended.
> 
> I think it's more sensible to do the shift first to a
> temporary then direct comparisons.
> 
> Perhaps something like this cleanup that also uses
> a temporary for aiptek->curSetting and
> !!(foo & mask) instead of ((foo & mask) != 0) ? 1 : 0;

Unless you have the device I'd rather kept the changes (which are mostly
cosmetic in nature and do not fix any bugs) to a minimum.

Thanks.

-- 
Dmitry

Re: [PATCH 03/11] aiptek: Fix probable mask then right shift defects

[Joe Perches]

On Mon, 2014-10-27 at 11:01 -0700, Dmitry Torokhov wrote:
> On Monday, October 27, 2014 10:56:54 AM Joe Perches wrote:
> > On Mon, 2014-10-27 at 07:44 -0700, Dmitry Torokhov wrote:
> > > On Sun, Oct 26, 2014 at 10:24:59PM -0700, Joe Perches wrote:
> > > > Precedence of & and >> is not the same and is not left to right.
> > > > shift has higher precedence and should be done after the mask.
> > > 
> > > Looking at the protocol description the current code is exactly right.
> > > We want to "move" button bits first as in packet type 1 they are in a
> > > different place than in other packets.
> > > 
> > > I'll take a patch that adds parenthesis around shifts to make clear it
> > > is intended.
> > 
> > I think it's more sensible to do the shift first to a
> > temporary then direct comparisons.
[]
> Unless you have the device I'd rather kept the changes (which are mostly
> cosmetic in nature and do not fix any bugs) to a minimum.

I don't have the device.
I think you should do what you think appropriate.

cheers, Joe

[PATCH 04/11] dvb-net: Fix probable mask then right shift defects

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the mask.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/media/dvb-core/dvb_net.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index 059e611..441814b 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -379,7 +379,9 @@ static void dvb_net_ule( struct net_device *dev, const u8 *buf, size_t buf_len )
 			/* Check TS error conditions: sync_byte, transport_error_indicator, scrambling_control . */
 			if ((ts[0] != TS_SYNC) || (ts[1] & TS_TEI) || ((ts[3] & TS_SC) != 0)) {
 				printk(KERN_WARNING "%lu: Invalid TS cell: SYNC %#x, TEI %u, SC %#x.\n",
-				       priv->ts_count, ts[0], ts[1] & TS_TEI >> 7, ts[3] & 0xC0 >> 6);
+				       priv->ts_count, ts[0],
+				       (ts[1] & TS_TEI) >> 7,
+				       (ts[3] & 0xC0) >> 6);
 
 				/* Drop partly decoded SNDU, reset state, resync on PUSI. */
 				if (priv->ule_skb) {
-- 
2.1.2

[PATCH 05/11] cx25840/cx18: Use standard ordering of mask and shift

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

This use has a mask then shift which is not the normal style.

Move the shift before the mask to match nearly all the other
uses in kernel.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/media/i2c/cx25840/cx25840-core.c | 12 ++++++------
 drivers/media/pci/cx18/cx18-av-core.c    | 16 ++++++++--------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
index e453a3f..0327032 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -879,7 +879,7 @@ void cx25840_std_setup(struct i2c_client *client)
 	/* Sets horizontal blanking delay and active lines */
 	cx25840_write(client, 0x470, hblank);
 	cx25840_write(client, 0x471,
-			0xff & (((hblank >> 8) & 0x3) | (hactive << 4)));
+		      (((hblank >> 8) & 0x3) | (hactive << 4)) & 0xff);
 	cx25840_write(client, 0x472, hactive >> 4);
 
 	/* Sets burst gate delay */
@@ -888,13 +888,13 @@ void cx25840_std_setup(struct i2c_client *client)
 	/* Sets vertical blanking delay and active duration */
 	cx25840_write(client, 0x474, vblank);
 	cx25840_write(client, 0x475,
-			0xff & (((vblank >> 8) & 0x3) | (vactive << 4)));
+		      (((vblank >> 8) & 0x3) | (vactive << 4)) & 0xff);
 	cx25840_write(client, 0x476, vactive >> 4);
 	cx25840_write(client, 0x477, vblank656);
 
 	/* Sets src decimation rate */
-	cx25840_write(client, 0x478, 0xff & src_decimation);
-	cx25840_write(client, 0x479, 0xff & (src_decimation >> 8));
+	cx25840_write(client, 0x478, src_decimation & 0xff);
+	cx25840_write(client, 0x479, (src_decimation >> 8) & 0xff);
 
 	/* Sets Luma and UV Low pass filters */
 	cx25840_write(client, 0x47a, luma_lpf << 6 | ((uv_lpf << 4) & 0x30));
@@ -904,8 +904,8 @@ void cx25840_std_setup(struct i2c_client *client)
 
 	/* Sets SC Step*/
 	cx25840_write(client, 0x47c, sc);
-	cx25840_write(client, 0x47d, 0xff & sc >> 8);
-	cx25840_write(client, 0x47e, 0xff & sc >> 16);
+	cx25840_write(client, 0x47d, (sc >> 8) & 0xff);
+	cx25840_write(client, 0x47e, (sc >> 16) & 0xff);
 
 	/* Sets VBI parameters */
 	if (std & V4L2_STD_625_50) {
diff --git a/drivers/media/pci/cx18/cx18-av-core.c b/drivers/media/pci/cx18/cx18-av-core.c
index 2d3afe0..45be26c 100644
--- a/drivers/media/pci/cx18/cx18-av-core.c
+++ b/drivers/media/pci/cx18/cx18-av-core.c
@@ -490,8 +490,8 @@ void cx18_av_std_setup(struct cx18 *cx)
 
 	/* Sets horizontal blanking delay and active lines */
 	cx18_av_write(cx, 0x470, hblank);
-	cx18_av_write(cx, 0x471, 0xff & (((hblank >> 8) & 0x3) |
-						(hactive << 4)));
+	cx18_av_write(cx, 0x471,
+		      (((hblank >> 8) & 0x3) | (hactive << 4)) & 0xff);
 	cx18_av_write(cx, 0x472, hactive >> 4);
 
 	/* Sets burst gate delay */
@@ -499,14 +499,14 @@ void cx18_av_std_setup(struct cx18 *cx)
 
 	/* Sets vertical blanking delay and active duration */
 	cx18_av_write(cx, 0x474, vblank);
-	cx18_av_write(cx, 0x475, 0xff & (((vblank >> 8) & 0x3) |
-						(vactive << 4)));
+	cx18_av_write(cx, 0x475,
+		      (((vblank >> 8) & 0x3) | (vactive << 4)) & 0xff);
 	cx18_av_write(cx, 0x476, vactive >> 4);
 	cx18_av_write(cx, 0x477, vblank656);
 
 	/* Sets src decimation rate */
-	cx18_av_write(cx, 0x478, 0xff & src_decimation);
-	cx18_av_write(cx, 0x479, 0xff & (src_decimation >> 8));
+	cx18_av_write(cx, 0x478, src_decimation & 0xff);
+	cx18_av_write(cx, 0x479, (src_decimation >> 8) & 0xff);
 
 	/* Sets Luma and UV Low pass filters */
 	cx18_av_write(cx, 0x47a, luma_lpf << 6 | ((uv_lpf << 4) & 0x30));
@@ -516,8 +516,8 @@ void cx18_av_std_setup(struct cx18 *cx)
 
 	/* Sets SC Step*/
 	cx18_av_write(cx, 0x47c, sc);
-	cx18_av_write(cx, 0x47d, 0xff & sc >> 8);
-	cx18_av_write(cx, 0x47e, 0xff & sc >> 16);
+	cx18_av_write(cx, 0x47d, (sc >> 8) & 0xff);
+	cx18_av_write(cx, 0x47e, (sc >> 16) & 0xff);
 
 	if (std & V4L2_STD_625_50) {
 		state->slicer_line_delay = 1;
-- 
2.1.2

Re: [PATCH 05/11] cx25840/cx18: Use standard ordering of mask and shift

[Andy Walls]

On Sun, 2014-10-26 at 22:25 -0700, Joe Perches wrote:
> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
> 
> This use has a mask then shift which is not the normal style.
> 
> Move the shift before the mask to match nearly all the other
> uses in kernel.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

The patch is technically correct.

Reviewed-by: Andy Walls <awalls@md.metrocast.net>

> ---
>  drivers/media/i2c/cx25840/cx25840-core.c | 12 ++++++------
>  drivers/media/pci/cx18/cx18-av-core.c    | 16 ++++++++--------
>  2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
> index e453a3f..0327032 100644
> --- a/drivers/media/i2c/cx25840/cx25840-core.c
> +++ b/drivers/media/i2c/cx25840/cx25840-core.c
> @@ -879,7 +879,7 @@ void cx25840_std_setup(struct i2c_client *client)
>  	/* Sets horizontal blanking delay and active lines */
>  	cx25840_write(client, 0x470, hblank);
>  	cx25840_write(client, 0x471,
> -			0xff & (((hblank >> 8) & 0x3) | (hactive << 4)));
> +		      (((hblank >> 8) & 0x3) | (hactive << 4)) & 0xff);
>  	cx25840_write(client, 0x472, hactive >> 4);
>  
>  	/* Sets burst gate delay */
> @@ -888,13 +888,13 @@ void cx25840_std_setup(struct i2c_client *client)
>  	/* Sets vertical blanking delay and active duration */
>  	cx25840_write(client, 0x474, vblank);
>  	cx25840_write(client, 0x475,
> -			0xff & (((vblank >> 8) & 0x3) | (vactive << 4)));
> +		      (((vblank >> 8) & 0x3) | (vactive << 4)) & 0xff);
>  	cx25840_write(client, 0x476, vactive >> 4);
>  	cx25840_write(client, 0x477, vblank656);
>  
>  	/* Sets src decimation rate */
> -	cx25840_write(client, 0x478, 0xff & src_decimation);
> -	cx25840_write(client, 0x479, 0xff & (src_decimation >> 8));
> +	cx25840_write(client, 0x478, src_decimation & 0xff);
> +	cx25840_write(client, 0x479, (src_decimation >> 8) & 0xff);
>  
>  	/* Sets Luma and UV Low pass filters */
>  	cx25840_write(client, 0x47a, luma_lpf << 6 | ((uv_lpf << 4) & 0x30));
> @@ -904,8 +904,8 @@ void cx25840_std_setup(struct i2c_client *client)
>  
>  	/* Sets SC Step*/
>  	cx25840_write(client, 0x47c, sc);
> -	cx25840_write(client, 0x47d, 0xff & sc >> 8);
> -	cx25840_write(client, 0x47e, 0xff & sc >> 16);
> +	cx25840_write(client, 0x47d, (sc >> 8) & 0xff);
> +	cx25840_write(client, 0x47e, (sc >> 16) & 0xff);
>  
>  	/* Sets VBI parameters */
>  	if (std & V4L2_STD_625_50) {
> diff --git a/drivers/media/pci/cx18/cx18-av-core.c b/drivers/media/pci/cx18/cx18-av-core.c
> index 2d3afe0..45be26c 100644
> --- a/drivers/media/pci/cx18/cx18-av-core.c
> +++ b/drivers/media/pci/cx18/cx18-av-core.c
> @@ -490,8 +490,8 @@ void cx18_av_std_setup(struct cx18 *cx)
>  
>  	/* Sets horizontal blanking delay and active lines */
>  	cx18_av_write(cx, 0x470, hblank);
> -	cx18_av_write(cx, 0x471, 0xff & (((hblank >> 8) & 0x3) |
> -						(hactive << 4)));
> +	cx18_av_write(cx, 0x471,
> +		      (((hblank >> 8) & 0x3) | (hactive << 4)) & 0xff);
>  	cx18_av_write(cx, 0x472, hactive >> 4);
>  
>  	/* Sets burst gate delay */
> @@ -499,14 +499,14 @@ void cx18_av_std_setup(struct cx18 *cx)
>  
>  	/* Sets vertical blanking delay and active duration */
>  	cx18_av_write(cx, 0x474, vblank);
> -	cx18_av_write(cx, 0x475, 0xff & (((vblank >> 8) & 0x3) |
> -						(vactive << 4)));
> +	cx18_av_write(cx, 0x475,
> +		      (((vblank >> 8) & 0x3) | (vactive << 4)) & 0xff);
>  	cx18_av_write(cx, 0x476, vactive >> 4);
>  	cx18_av_write(cx, 0x477, vblank656);
>  
>  	/* Sets src decimation rate */
> -	cx18_av_write(cx, 0x478, 0xff & src_decimation);
> -	cx18_av_write(cx, 0x479, 0xff & (src_decimation >> 8));
> +	cx18_av_write(cx, 0x478, src_decimation & 0xff);
> +	cx18_av_write(cx, 0x479, (src_decimation >> 8) & 0xff);
>  
>  	/* Sets Luma and UV Low pass filters */
>  	cx18_av_write(cx, 0x47a, luma_lpf << 6 | ((uv_lpf << 4) & 0x30));
> @@ -516,8 +516,8 @@ void cx18_av_std_setup(struct cx18 *cx)
>  
>  	/* Sets SC Step*/
>  	cx18_av_write(cx, 0x47c, sc);
> -	cx18_av_write(cx, 0x47d, 0xff & sc >> 8);
> -	cx18_av_write(cx, 0x47e, 0xff & sc >> 16);
> +	cx18_av_write(cx, 0x47d, (sc >> 8) & 0xff);
> +	cx18_av_write(cx, 0x47e, (sc >> 16) & 0xff);
>  
>  	if (std & V4L2_STD_625_50) {
>  		state->slicer_line_delay = 1;

[PATCH 06/11] wm8350-core: Fix probable mask then right shift defect

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the mask.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/mfd/wm8350-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/wm8350-core.c b/drivers/mfd/wm8350-core.c
index 4ab527f..f5124a8 100644
--- a/drivers/mfd/wm8350-core.c
+++ b/drivers/mfd/wm8350-core.c
@@ -308,7 +308,7 @@ int wm8350_device_init(struct wm8350 *wm8350, int irq,
 		goto err;
 	}
 
-	mode = id2 & WM8350_CONF_STS_MASK >> 10;
+	mode = (id2 & WM8350_CONF_STS_MASK) >> 10;
 	cust_id = id2 & WM8350_CUST_ID_MASK;
 	chip_rev = (id2 & WM8350_CHIP_REV_MASK) >> 12;
 	dev_info(wm8350->dev,
-- 
2.1.2

Re: [PATCH 06/11] wm8350-core: Fix probable mask then right shift defect

[Charles Keepax]

On Sun, Oct 26, 2014 at 10:25:02PM -0700, Joe Perches wrote:
> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
> 
> Add parentheses around the mask.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

Re: [PATCH 06/11] wm8350-core: Fix probable mask then right shift defect

[Lee Jones]

On Sun, 26 Oct 2014, Joe Perches wrote:

> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
> 
> Add parentheses around the mask.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/mfd/wm8350-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied with Charles' Ack.

> diff --git a/drivers/mfd/wm8350-core.c b/drivers/mfd/wm8350-core.c
> index 4ab527f..f5124a8 100644
> --- a/drivers/mfd/wm8350-core.c
> +++ b/drivers/mfd/wm8350-core.c
> @@ -308,7 +308,7 @@ int wm8350_device_init(struct wm8350 *wm8350, int irq,
>  		goto err;
>  	}
>  
> -	mode = id2 & WM8350_CONF_STS_MASK >> 10;
> +	mode = (id2 & WM8350_CONF_STS_MASK) >> 10;
>  	cust_id = id2 & WM8350_CUST_ID_MASK;
>  	chip_rev = (id2 & WM8350_CHIP_REV_MASK) >> 12;
>  	dev_info(wm8350->dev,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 07/11] iwlwifi: dvm: Fix probable mask then right shift defect

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the mask.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/wireless/iwlwifi/dvm/lib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/dvm/lib.c b/drivers/net/wireless/iwlwifi/dvm/lib.c
index 2191621..02e4ede 100644
--- a/drivers/net/wireless/iwlwifi/dvm/lib.c
+++ b/drivers/net/wireless/iwlwifi/dvm/lib.c
@@ -418,8 +418,8 @@ void iwlagn_bt_adjust_rssi_monitor(struct iwl_priv *priv, bool rssi_ena)
 
 static bool iwlagn_bt_traffic_is_sco(struct iwl_bt_uart_msg *uart_msg)
 {
-	return BT_UART_MSG_FRAME3SCOESCO_MSK & uart_msg->frame3 >>
-			BT_UART_MSG_FRAME3SCOESCO_POS;
+	return (BT_UART_MSG_FRAME3SCOESCO_MSK & uart_msg->frame3) >>
+		BT_UART_MSG_FRAME3SCOESCO_POS;
 }
 
 static void iwlagn_bt_traffic_change_work(struct work_struct *work)
-- 
2.1.2

RE: [PATCH 07/11] iwlwifi: dvm: Fix probable mask then right shift defect

[Grumbach, Emmanuel]

> 
> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
> 
> Add parentheses around the mask.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---

Applied - thanks.

[PATCH 08/11] ssb: driver_chip_comon_pmu: Fix probable mask then right shift defect

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the mask.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/ssb/driver_chipcommon_pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ssb/driver_chipcommon_pmu.c b/drivers/ssb/driver_chipcommon_pmu.c
index 1173a09..bc71583 100644
--- a/drivers/ssb/driver_chipcommon_pmu.c
+++ b/drivers/ssb/driver_chipcommon_pmu.c
@@ -621,8 +621,8 @@ static u32 ssb_pmu_get_alp_clock_clk0(struct ssb_chipcommon *cc)
 	u32 crystalfreq;
 	const struct pmu0_plltab_entry *e = NULL;
 
-	crystalfreq = chipco_read32(cc, SSB_CHIPCO_PMU_CTL) &
-		      SSB_CHIPCO_PMU_CTL_XTALFREQ >> SSB_CHIPCO_PMU_CTL_XTALFREQ_SHIFT;
+	crystalfreq = (chipco_read32(cc, SSB_CHIPCO_PMU_CTL) &
+		       SSB_CHIPCO_PMU_CTL_XTALFREQ) >> SSB_CHIPCO_PMU_CTL_XTALFREQ_SHIFT;
 	e = pmu0_plltab_find_entry(crystalfreq);
 	BUG_ON(!e);
 	return e->freq * 1000;
-- 
2.1.2

Re: [PATCH 08/11] ssb: driver_chip_comon_pmu: Fix probable mask then right shift defect

[Michael Büsch]

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

On Sun, 26 Oct 2014 22:25:04 -0700
Joe Perches <joe@perches.com> wrote:

> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
> 
> Add parentheses around the mask.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Good catch.

Reviewed-by: Michael Büsch <m@bues.ch>

> ---
>  drivers/ssb/driver_chipcommon_pmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ssb/driver_chipcommon_pmu.c b/drivers/ssb/driver_chipcommon_pmu.c
> index 1173a09..bc71583 100644
> --- a/drivers/ssb/driver_chipcommon_pmu.c
> +++ b/drivers/ssb/driver_chipcommon_pmu.c
> @@ -621,8 +621,8 @@ static u32 ssb_pmu_get_alp_clock_clk0(struct ssb_chipcommon *cc)
>  	u32 crystalfreq;
>  	const struct pmu0_plltab_entry *e = NULL;
>  
> -	crystalfreq = chipco_read32(cc, SSB_CHIPCO_PMU_CTL) &
> -		      SSB_CHIPCO_PMU_CTL_XTALFREQ >> SSB_CHIPCO_PMU_CTL_XTALFREQ_SHIFT;
> +	crystalfreq = (chipco_read32(cc, SSB_CHIPCO_PMU_CTL) &
> +		       SSB_CHIPCO_PMU_CTL_XTALFREQ) >> SSB_CHIPCO_PMU_CTL_XTALFREQ_SHIFT;
>  	e = pmu0_plltab_find_entry(crystalfreq);
>  	BUG_ON(!e);
>  	return e->freq * 1000;




-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 09/11] tty: ipwireless: Fix probable mask then right shift defects

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the masks.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/tty/ipwireless/hardware.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/ipwireless/hardware.c b/drivers/tty/ipwireless/hardware.c
index 2c14842..5c77e1e 100644
--- a/drivers/tty/ipwireless/hardware.c
+++ b/drivers/tty/ipwireless/hardware.c
@@ -378,9 +378,9 @@ static void swap_packet_bitfield_to_le(unsigned char *data)
 	/*
 	 * transform bits from aa.bbb.ccc to ccc.bbb.aa
 	 */
-	ret |= tmp & 0xc0 >> 6;
-	ret |= tmp & 0x38 >> 1;
-	ret |= tmp & 0x07 << 5;
+	ret |= (tmp & 0xc0) >> 6;
+	ret |= (tmp & 0x38) >> 1;
+	ret |= (tmp & 0x07) << 5;
 	*data = ret & 0xff;
 #endif
 }
@@ -393,9 +393,9 @@ static void swap_packet_bitfield_from_le(unsigned char *data)
 	/*
 	 * transform bits from ccc.bbb.aa to aa.bbb.ccc
 	 */
-	ret |= tmp & 0xe0 >> 5;
-	ret |= tmp & 0x1c << 1;
-	ret |= tmp & 0x03 << 6;
+	ret |= (tmp & 0xe0) >> 5;
+	ret |= (tmp & 0x1c) << 1;
+	ret |= (tmp & 0x03) << 6;
 	*data = ret & 0xff;
 #endif
 }
-- 
2.1.2

Re: [PATCH 09/11] tty: ipwireless: Fix probable mask then right shift defects

[David Sterba]

On Sun, Oct 26, 2014 at 10:25:05PM -0700, Joe Perches wrote:
> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
> 
> Add parentheses around the masks.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Acked-by: David Sterba <dsterba@suse.cz>

Thanks.

Re: [PATCH 09/11] tty: ipwireless: Fix probable mask then right shift defects

[Jiri Kosina]

On Sun, 26 Oct 2014, Joe Perches wrote:

> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
> 
> Add parentheses around the masks.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Reviewed-by: Jiri Kosina <jkosina@suse.cz>

Greg, can you take this through your char/misc tree, please?

Thanks.

> ---
>  drivers/tty/ipwireless/hardware.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/ipwireless/hardware.c b/drivers/tty/ipwireless/hardware.c
> index 2c14842..5c77e1e 100644
> --- a/drivers/tty/ipwireless/hardware.c
> +++ b/drivers/tty/ipwireless/hardware.c
> @@ -378,9 +378,9 @@ static void swap_packet_bitfield_to_le(unsigned char *data)
>  	/*
>  	 * transform bits from aa.bbb.ccc to ccc.bbb.aa
>  	 */
> -	ret |= tmp & 0xc0 >> 6;
> -	ret |= tmp & 0x38 >> 1;
> -	ret |= tmp & 0x07 << 5;
> +	ret |= (tmp & 0xc0) >> 6;
> +	ret |= (tmp & 0x38) >> 1;
> +	ret |= (tmp & 0x07) << 5;
>  	*data = ret & 0xff;
>  #endif
>  }
> @@ -393,9 +393,9 @@ static void swap_packet_bitfield_from_le(unsigned char *data)
>  	/*
>  	 * transform bits from ccc.bbb.aa to aa.bbb.ccc
>  	 */
> -	ret |= tmp & 0xe0 >> 5;
> -	ret |= tmp & 0x1c << 1;
> -	ret |= tmp & 0x03 << 6;
> +	ret |= (tmp & 0xe0) >> 5;
> +	ret |= (tmp & 0x1c) << 1;
> +	ret |= (tmp & 0x03) << 6;
>  	*data = ret & 0xff;
>  #endif
>  }
> -- 
> 2.1.2
> 

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 09/11] tty: ipwireless: Fix probable mask then right shift defects

[Greg Kroah-Hartman]

On Mon, Oct 27, 2014 at 03:17:28PM +0100, Jiri Kosina wrote:
> On Sun, 26 Oct 2014, Joe Perches wrote:
> 
> > Precedence of & and >> is not the same and is not left to right.
> > shift has higher precedence and should be done after the mask.
> > 
> > Add parentheses around the masks.
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> 
> Reviewed-by: Jiri Kosina <jkosina@suse.cz>
> 
> Greg, can you take this through your char/misc tree, please?

Ok, will do, give me a week or so to catch up on pending patches.

greg k-h

[PATCH 10/11] hwa-hc: Fix probable mask then right shift defect

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the mask.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/usb/host/hwa-hc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/hwa-hc.c b/drivers/usb/host/hwa-hc.c
index d0d8fad..1db0626 100644
--- a/drivers/usb/host/hwa-hc.c
+++ b/drivers/usb/host/hwa-hc.c
@@ -607,7 +607,7 @@ found:
 	wa->wa_descr = wa_descr = (struct usb_wa_descriptor *) hdr;
 	if (le16_to_cpu(wa_descr->bcdWAVersion) > 0x0100)
 		dev_warn(dev, "Wire Adapter v%d.%d newer than groked v1.0\n",
-			 le16_to_cpu(wa_descr->bcdWAVersion) & 0xff00 >> 8,
+			 (le16_to_cpu(wa_descr->bcdWAVersion) & 0xff00) >> 8,
 			 le16_to_cpu(wa_descr->bcdWAVersion) & 0x00ff);
 	result = 0;
 error:
-- 
2.1.2

[PATCH 11/11] sound: ad1889: Fix probable mask then right shift defects

[Joe Perches]

Precedence of & and >> is not the same and is not left to right.
shift has higher precedence and should be done after the mask.

Add parentheses around the mask.

Signed-off-by: Joe Perches <joe@perches.com>
---
 sound/pci/ad1889.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/pci/ad1889.c b/sound/pci/ad1889.c
index 7bfdf9c..1610c38 100644
--- a/sound/pci/ad1889.c
+++ b/sound/pci/ad1889.c
@@ -681,7 +681,7 @@ snd_ad1889_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffe
 	
 	/* WARQ is at offset 12 */
 	tmp = (reg & AD_DS_WSMC_WARQ) ?
-			(((reg & AD_DS_WSMC_WARQ >> 12) & 0x01) ? 12 : 18) : 4;
+		((((reg & AD_DS_WSMC_WARQ) >> 12) & 0x01) ? 12 : 18) : 4;
 	tmp /= (reg & AD_DS_WSMC_WAST) ? 2 : 1;
 	
 	snd_iprintf(buffer, "Wave FIFO: %d %s words\n\n", tmp,
@@ -693,7 +693,7 @@ snd_ad1889_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffe
 	
 	/* SYRQ is at offset 4 */
 	tmp = (reg & AD_DS_WSMC_SYRQ) ?
-			(((reg & AD_DS_WSMC_SYRQ >> 4) & 0x01) ? 12 : 18) : 4;
+		((((reg & AD_DS_WSMC_SYRQ) >> 4) & 0x01) ? 12 : 18) : 4;
 	tmp /= (reg & AD_DS_WSMC_WAST) ? 2 : 1;
 	
 	snd_iprintf(buffer, "Synthesis FIFO: %d %s words\n\n", tmp,
@@ -709,7 +709,7 @@ snd_ad1889_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffe
 	
 	/* ACRQ is at offset 4 */
 	tmp = (reg & AD_DS_RAMC_ACRQ) ?
-			(((reg & AD_DS_RAMC_ACRQ >> 4) & 0x01) ? 12 : 18) : 4;
+		((((reg & AD_DS_RAMC_ACRQ) >> 4) & 0x01) ? 12 : 18) : 4;
 	tmp /= (reg & AD_DS_RAMC_ADST) ? 2 : 1;
 	
 	snd_iprintf(buffer, "ADC FIFO: %d %s words\n\n", tmp,
@@ -720,7 +720,7 @@ snd_ad1889_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffe
 			
 	/* RERQ is at offset 12 */
 	tmp = (reg & AD_DS_RAMC_RERQ) ?
-			(((reg & AD_DS_RAMC_RERQ >> 12) & 0x01) ? 12 : 18) : 4;
+		((((reg & AD_DS_RAMC_RERQ) >> 12) & 0x01) ? 12 : 18) : 4;
 	tmp /= (reg & AD_DS_RAMC_ADST) ? 2 : 1;
 	
 	snd_iprintf(buffer, "Resampler FIFO: %d %s words\n\n", tmp,
-- 
2.1.2

Re: [PATCH 11/11] sound: ad1889: Fix probable mask then right shift defects

[Takashi Iwai]

At Sun, 26 Oct 2014 22:25:07 -0700,
Joe Perches wrote:
> 
> Precedence of & and >> is not the same and is not left to right.
> shift has higher precedence and should be done after the mask.
> 
> Add parentheses around the mask.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Thanks, applied.


Takashi


> ---
>  sound/pci/ad1889.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/pci/ad1889.c b/sound/pci/ad1889.c
> index 7bfdf9c..1610c38 100644
> --- a/sound/pci/ad1889.c
> +++ b/sound/pci/ad1889.c
> @@ -681,7 +681,7 @@ snd_ad1889_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffe
>  	
>  	/* WARQ is at offset 12 */
>  	tmp = (reg & AD_DS_WSMC_WARQ) ?
> -			(((reg & AD_DS_WSMC_WARQ >> 12) & 0x01) ? 12 : 18) : 4;
> +		((((reg & AD_DS_WSMC_WARQ) >> 12) & 0x01) ? 12 : 18) : 4;
>  	tmp /= (reg & AD_DS_WSMC_WAST) ? 2 : 1;
>  	
>  	snd_iprintf(buffer, "Wave FIFO: %d %s words\n\n", tmp,
> @@ -693,7 +693,7 @@ snd_ad1889_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffe
>  	
>  	/* SYRQ is at offset 4 */
>  	tmp = (reg & AD_DS_WSMC_SYRQ) ?
> -			(((reg & AD_DS_WSMC_SYRQ >> 4) & 0x01) ? 12 : 18) : 4;
> +		((((reg & AD_DS_WSMC_SYRQ) >> 4) & 0x01) ? 12 : 18) : 4;
>  	tmp /= (reg & AD_DS_WSMC_WAST) ? 2 : 1;
>  	
>  	snd_iprintf(buffer, "Synthesis FIFO: %d %s words\n\n", tmp,
> @@ -709,7 +709,7 @@ snd_ad1889_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffe
>  	
>  	/* ACRQ is at offset 4 */
>  	tmp = (reg & AD_DS_RAMC_ACRQ) ?
> -			(((reg & AD_DS_RAMC_ACRQ >> 4) & 0x01) ? 12 : 18) : 4;
> +		((((reg & AD_DS_RAMC_ACRQ) >> 4) & 0x01) ? 12 : 18) : 4;
>  	tmp /= (reg & AD_DS_RAMC_ADST) ? 2 : 1;
>  	
>  	snd_iprintf(buffer, "ADC FIFO: %d %s words\n\n", tmp,
> @@ -720,7 +720,7 @@ snd_ad1889_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffe
>  			
>  	/* RERQ is at offset 12 */
>  	tmp = (reg & AD_DS_RAMC_RERQ) ?
> -			(((reg & AD_DS_RAMC_RERQ >> 12) & 0x01) ? 12 : 18) : 4;
> +		((((reg & AD_DS_RAMC_RERQ) >> 12) & 0x01) ? 12 : 18) : 4;
>  	tmp /= (reg & AD_DS_RAMC_ADST) ? 2 : 1;
>  	
>  	snd_iprintf(buffer, "Resampler FIFO: %d %s words\n\n", tmp,
> -- 
> 2.1.2
>