Re: [PATCH v2 2/2] spi: imx: support word delay

[Kees Bakker]

Op 13-11-2024 om 13:18 schreef Jonas Rebmann:
> Implement support for the word delay feature of i.MX51 (and onwards) via
> the ECSPI interface.
>
> Convert the requested delay to SPI cycles and account for an extra
> inter-word delay inserted by the controller in addition to the requested
> number of cycles, which was observed when testing this patch.
>
> Disable dynamic burst when word delay is set. As the configurable delay
> period in the controller is inserted after bursts, the burst length must
> equal the word length.
>
> Account for word delay in the transfer time estimation for
> polling_limit_us.
>
> Signed-off-by: Jonas Rebmann <jre@xxxxxxxxxxxxxx>
> ---
>   drivers/spi/spi-imx.c | 95 +++++++++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 85 insertions(+), 10 deletions(-)
>
> [...]
> +static unsigned int spi_imx_transfer_estimate_time_us(struct spi_transfer *transfer)
> +{
> +	u64 result;
> +
> +	result = DIV_U64_ROUND_CLOSEST((u64)USEC_PER_SEC * transfer->len * BITS_PER_BYTE,
> +				       transfer->effective_speed_hz);
> +	if (transfer->word_delay.value) {
> +		unsigned int word_delay_us;
> +		unsigned int words;
> +
> +		words = DIV_ROUND_UP(transfer->len * BITS_PER_BYTE, transfer->bits_per_word);
> +		word_delay_us = DIV_ROUND_CLOSEST(spi_delay_to_ns(&transfer->word_delay, transfer),
> +						  NSEC_PER_USEC);
> +		result += words * word_delay_us;
If the multiplication can overflow 32 bits to need to force a 64 bits 
multiply.
    result += (u64)words * word_delay_us;

But I'm wondering if `result` needs to be u64.
> +	}
> +
> +	return min(result, U32_MAX);
Do you really expect this much? You're clipping to U32_MAX.
U32_MAX microsecs is already more than an hour.
> +}
> [...]