The name of the function is somewhat misleading, it's not just XORing
bits, but is calculating the parity of the passed in value. There's
already a compiler builtin function for this - __builtin_parity. Just
use it. No functional changes.

Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
---
 drivers/ras/amd/atl/umc.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/ras/amd/atl/umc.c b/drivers/ras/amd/atl/umc.c
index 6e072b7667e9..7ff4a5a1c5da 100644
--- a/drivers/ras/amd/atl/umc.c
+++ b/drivers/ras/amd/atl/umc.c
@@ -49,18 +49,6 @@ static u8 get_coh_st_inst_id_mi300(struct atl_err *err)
 	return i;
 }
 
-/* XOR the bits in @val. */
-static u16 bitwise_xor_bits(u16 val)
-{
-	u16 tmp = 0;
-	u8 i;
-
-	for (i = 0; i < 16; i++)
-		tmp ^= (val >> i) & 0x1;
-
-	return tmp;
-}
-
 struct xor_bits {
 	bool	xor_enable;
 	u16	col_xor;
@@ -250,17 +238,17 @@ static unsigned long convert_dram_to_norm_addr_mi300(unsigned long addr)
 		if (!addr_hash.bank[i].xor_enable)
 			continue;
 
-		temp  = bitwise_xor_bits(col & addr_hash.bank[i].col_xor);
-		temp ^= bitwise_xor_bits(row & addr_hash.bank[i].row_xor);
+		temp  = (u16)__builtin_parity(col & addr_hash.bank[i].col_xor);
+		temp ^= (u16)__builtin_parity(row & addr_hash.bank[i].row_xor);
 		bank ^= temp << i;
 	}
 
 	/* Calculate hash for PC bit. */
 	if (addr_hash.pc.xor_enable) {
-		temp  = bitwise_xor_bits(col  & addr_hash.pc.col_xor);
-		temp ^= bitwise_xor_bits(row  & addr_hash.pc.row_xor);
+		temp  = (u16)__builtin_parity(col & addr_hash.pc.col_xor);
+		temp ^= (u16)__builtin_parity(row & addr_hash.pc.row_xor);
 		/* Bits SID[1:0] act as Bank[5:4] for PC hash, so apply them here. */
-		temp ^= bitwise_xor_bits((bank | sid << NUM_BANK_BITS) & addr_hash.bank_xor);
+		temp ^= (u16)__builtin_parity((bank | sid << NUM_BANK_BITS) & addr_hash.bank_xor);
 		pc   ^= temp;
 	}
 
-- 
2.51.1

On Thu, Nov 13, 2025 at 01:11:25PM +0200, Nikolay Borisov wrote:
> The name of the function is somewhat misleading, it's not just XORing
> bits, but is calculating the parity of the passed in value.

Huh?

It is bitwise XORing the bits. That just *happens* to be the same thing as
calculating the parity modulo 2.

Unless you wanna say something else here which I don't grok...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

On 11/18/25 14:33, Borislav Petkov wrote:
> It is bitwise XORing the bits. That just*happens* to be the same thing as
> calculating the parity modulo 2.
> 
> Unless you wanna say something else here which I don't grok...

Fair point, but bitwise xor is the lowest possible operation, I guess we 
care about the higher-level effect, which is calculating parity.

>

On Tue, Nov 18, 2025 at 02:36:08PM +0200, Nikolay Borisov wrote:
> Fair point, but bitwise xor is the lowest possible operation, I guess we
> care about the higher-level effect, which is calculating parity.

... in a function which "Calculate hash for each Bank bit."?

I don't think it matters either way. This is converting the DRAM addressing
scheme into normalized addresses so as long as it is clear what happens
there...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

On 11/20/25 17:41, Borislav Petkov wrote:
> On Tue, Nov 18, 2025 at 02:36:08PM +0200, Nikolay Borisov wrote:
>> Fair point, but bitwise xor is the lowest possible operation, I guess we
>> care about the higher-level effect, which is calculating parity.
> 
> ... in a function which "Calculate hash for each Bank bit."?
> 
> I don't think it matters either way. This is converting the DRAM addressing
> scheme into normalized addresses so as long as it is clear what happens
> there...
> 


So are you saying the patch should be dropped altogether or that the 
changelog should be reworded?  The way I see it, the parity for the 
given bank's col/row are calculated and this then is used into 
calculating the hash.

On Thu, Nov 20, 2025 at 05:45:10PM +0200, Nikolay Borisov wrote:
> So are you saying the patch should be dropped altogether or that the
> changelog should be reworded?  The way I see it, the parity for the given
> bank's col/row are calculated and this then is used into calculating the
> hash.

For you it is the parity, for someone else it is XORing the bits. All boiling
down to the same operation.

The only upside to the change is the removal of the function and using the
builtin. Which begs the question, do all compilers support that builtin?

If yes, then you can reword and resend.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette