Re: [PATCH 2/2] can: arasan-canfd: add driver for arasan CAN-FD controller

On 03.08.26 16:05, Jisheng Zhang wrote:
> Add driver for the arasan CAN-FD controller.
> 

> +
> +static const u32 ecc_syndrom[7] = {
> +	0xC14840FF,
> +	0x2124FF90,
> +	0x6CFF0808,
> +	0xFF01A444,
> +	0x16F092A6,
> +	0x101F7161,
> +	0x8A820F1B,
> +};
> +
> +static inline int parity(unsigned int x)

int / unsigned int ?

u32 might have been better.

> +{
> +	/*
> +	 * public domain code snippet, lifted from
> +	 * http://www-graphics.stanford.edu/~seander/bithacks.html
> +	 */
> +	x ^= x >> 1;
> +	x ^= x >> 2;
> +	x = (x & 0x11111111U) * 0x11111111U;
> +	return (x >> 28) & 1;
> +}
> +
> +static u8 ecc_calc(u32 data)
> +{
> +	int i;
> +	u8 result = 0;
> +
> +	for (i = 0; i < 7; i++) {
> +		if (parity(data & ecc_syndrom[i]))
> +			result |= BIT(i);
> +	}
> +
> +	return result;
> +}
> +

IIRC there's a even better solution with already existing kernel 
functions (e.g. hweight32()) which can use optimized CPU operations e.g. 
to count bits in a value than open coding the parity function.

#include <linux/bitops.h>
#include <linux/types.h>

static const u32 ecc_syndrom[7] = {
	0xC14840FF,
	0x2124FF90,
	0x6CFF0808,
	0xFF01A444,
	0x16F092A6,
	0x101F7161,
	0x8A820F1B,
};

static u8 ecc_calc(u32 data)
{
	int i;
	u8 result = 0;

	for (i = 0; i < 7; i++) {
		if (hweight32(data & ecc_syndrom[i]) & 1)
			result |= BIT(i);
	}

	return result;
}

Please re-check if you can use existing kernel code here.

Best regards,
Oliver