Re: [PATCH RFC 5/7] spi: Add RX sampling point adjustment

[Frank Li]

From: Frank Li (AI-BOT) <frank.li@xxxxxxx>

> +unsigned int spi_calc_rx_sampling_point(struct spi_device *spi, unsigned int *freq,
> +					u16 max_delay_cycles,
> +					u16 delay_cycles_per_clock_cycle)
> +{
> +	unsigned long long temp;
> +	u16 delay_cycles;
> +
> +	/* if sampling delay is zero, we assume the default sampling point can be used */
> +	if (!spi->rx_sampling_delay_ns)
> +		return 0;
> +
> +	temp = *freq * delay_cycles_per_clock_cycle * spi->rx_sampling_delay_ns;
> +	do_div(temp, 1000000000UL);
> +	delay_cycles = temp;

Potential overflow: multiplying three u32/u16 values before assigning to
u64. Cast operands to u64 before multiplication to avoid intermediate
overflow.

> +	if (delay_cycles > max_delay_cycles) {
> +		/*
> +		 * Reduce the clock to the point where the sampling delay requirement
> +		 * can be met.
> +		 */
> +		delay_cycles = max_delay_cycles;
> +		temp = (1000000000UL * delay_cycles);
> +		do_div(temp, spi->rx_sampling_delay_ns * delay_cycles_per_clock_cycle);
> +		*freq = temp;

Same overflow risk in denominator multiplication. Cast to u64 first.

> +	dev_dbg(&spi->controller->dev, "calculated RX sampling point delay: %u cycle(s) at %lu KHz", delay_cycles, *freq / 1000);

Line exceeds 80 chars. Break into two lines.

> +unsigned int spi_set_rx_sampling_point(struct spi_device *spi, unsigned int freq)
> +{
> +	if (spi->controller->set_rx_sampling_point)
> +		return spi->controller->set_rx_sampling_point(spi, spi->max_speed_hz);
> +
> +	return freq;
> +}

Bug: function receives 'freq' parameter but ignores it and passes
'spi->max_speed_hz' to the handler instead. Should pass 'freq' or remove
the parameter.

> +	spi->max_speed_hz = spi_set_rx_sampling_point(spi, spi->max_speed_hz);

This call happens after spi_set_cs_timing() check but before the mutex is
unlocked. Verify this is the intended placement and that no locking issues
exist with controller callbacks.

AI bot review and may be useless.