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(-)
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;
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.
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;
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
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
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.
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Joe Perches -
Michel Dänzer