This error is reported by coverity scan stating as

CID 1593397: (#1 of 1): Unintended sign extension (SIGN_EXTENSION)
sign_extension: Suspicious implicit sign extension: pc
with type u16 (16 bits, unsigned) is promoted in
pc << bit_shifts.pc to type int (32 bits, signed),
then sign-extended to type unsigned long (64 bits, unsigned).
If pc << bit_shifts.pc is greater than 0x7FFFFFFF,
the upper bits of the result will all be 1.

Following the code styleof the file, assigning the u16
value to u32 variable and using it for the bit wise
operation, thus ensuring no unintentional sign
extension occurs.

Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com>
---
Coverity  Link: 
https://scan7.scan.coverity.com/#/project-view/51975/11354?selectedIssue=1593397
---
Changes in v2:
- Assigning pc value to temp variable before left shifting as mentioned
  in feedback rather then typecasting pc to u32. 
- Link to v1: https://lore.kernel.org/r/20241104-coverity1593397signextension-v1-1-4cfae6532140@gmail.com
---
 drivers/ras/amd/atl/umc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ras/amd/atl/umc.c b/drivers/ras/amd/atl/umc.c
index dc8aa12f63c8..3f4b1f31e14f 100644
--- a/drivers/ras/amd/atl/umc.c
+++ b/drivers/ras/amd/atl/umc.c
@@ -293,7 +293,8 @@ static unsigned long convert_dram_to_norm_addr_mi300(unsigned long addr)
 	}
 
 	/* PC bit */
-	addr |= pc << bit_shifts.pc;
+	temp = pc;
+	addr |= temp << bit_shifts.pc;
 
 	/* SID bits */
 	for (i = 0; i < NUM_SID_BITS; i++) {

---
base-commit: 81983758430957d9a5cb3333fe324fd70cf63e7e
change-id: 20241104-coverity1593397signextension-78c9b2c21d51

Best regards,
-- 
Karan Sanghavi <karansanghvi98@gmail.com>

On Fri, Nov 08, 2024 at 04:40:41PM +0000, Karan Sanghavi wrote:
> This error is reported by coverity scan stating as
> 
> CID 1593397: (#1 of 1): Unintended sign extension (SIGN_EXTENSION)
> sign_extension: Suspicious implicit sign extension: pc
> with type u16 (16 bits, unsigned) is promoted in
> pc << bit_shifts.pc to type int (32 bits, signed),
> then sign-extended to type unsigned long (64 bits, unsigned).
> If pc << bit_shifts.pc is greater than 0x7FFFFFFF,
> the upper bits of the result will all be 1.
> 
> Following the code styleof the file, assigning the u16

styleof -> style of

> value to u32 variable and using it for the bit wise
> operation, thus ensuring no unintentional sign
> extension occurs.
>

Please make sure you use an imperative voice here. For example, "assign
the value...and use it...". This should read like you are giving
commands.

> Signed-off-by: Karan Sanghavi <karansanghvi98@gmail.com>

Overall, looks good to me.

Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>

Thanks,
Yazen

> ---
> Coverity  Link: 
> https://scan7.scan.coverity.com/#/project-view/51975/11354?selectedIssue=1593397
> ---
> Changes in v2:
> - Assigning pc value to temp variable before left shifting as mentioned
>   in feedback rather then typecasting pc to u32. 
> - Link to v1: https://lore.kernel.org/r/20241104-coverity1593397signextension-v1-1-4cfae6532140@gmail.com
> ---
>  drivers/ras/amd/atl/umc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ras/amd/atl/umc.c b/drivers/ras/amd/atl/umc.c
> index dc8aa12f63c8..3f4b1f31e14f 100644
> --- a/drivers/ras/amd/atl/umc.c
> +++ b/drivers/ras/amd/atl/umc.c
> @@ -293,7 +293,8 @@ static unsigned long convert_dram_to_norm_addr_mi300(unsigned long addr)
>  	}
>  
>  	/* PC bit */
> -	addr |= pc << bit_shifts.pc;
> +	temp = pc;
> +	addr |= temp << bit_shifts.pc;
>  
>  	/* SID bits */
>  	for (i = 0; i < NUM_SID_BITS; i++) {
> 
> ---
> base-commit: 81983758430957d9a5cb3333fe324fd70cf63e7e
> change-id: 20241104-coverity1593397signextension-78c9b2c21d51
> 
> Best regards,
> -- 
> Karan Sanghavi <karansanghvi98@gmail.com>
>