Re: [PATCH v2 1/1] spi: pxa2xx: use min() instead of min_t()

[David Laight]

On Mon, 23 Feb 2026 16:30:54 +0100
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

> min_t(int, a, b) casts an 'u32' to 'int'. This might lead to
> the cases when big number is wrongly chosen. On the other hand,
> the SPI transfer speed rate is unsigned and driver uses signed type
> for an unknown reason. Change the type of the SPI transfer speed
> to be unsigned and convert to use min() instead of min_t().
> 

Reviewed-by: David Laight <david.laight.linux@xxxxxxxxx>
(but see below...)
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> 
> v2: used u32 instead of unsigned long (David)
> 
>  drivers/spi/spi-pxa2xx.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
> index f7881a31e4cc..6291d7c2e06f 100644
> --- a/drivers/spi/spi-pxa2xx.c
> +++ b/drivers/spi/spi-pxa2xx.c
> @@ -796,7 +796,7 @@ static irqreturn_t ssp_int(int irq, void *dev_id)
>   * The function calculates parameters for all cases and chooses the one closest
>   * to the asked baud rate.
>   */
> -static unsigned int quark_x1000_get_clk_div(int rate, u32 *dds)
> +static unsigned int quark_x1000_get_clk_div(u32 rate, u32 *dds)
>  {
>  	unsigned long xtal = 200000000;
>  	unsigned long fref = xtal / 2;		/* mandatory division by 2,

I've looked at the file...
I think all the 'long' in that function can be 32bit.
Especially if the code actually works on 32bit.
There is also the stunning:
	do_div(fssp, 1 << 24);
in the only bit that that does need u64.

	David


> @@ -885,12 +885,12 @@ static unsigned int quark_x1000_get_clk_div(int rate, u32 *dds)
>  	return q - 1;
>  }
>  
> -static unsigned int ssp_get_clk_div(struct driver_data *drv_data, int rate)
> +static unsigned int ssp_get_clk_div(struct driver_data *drv_data, u32 rate)
>  {
> -	unsigned long ssp_clk = drv_data->controller->max_speed_hz;
> +	u32 ssp_clk = drv_data->controller->max_speed_hz;
>  	const struct ssp_device *ssp = drv_data->ssp;
>  
> -	rate = min_t(int, ssp_clk, rate);
> +	rate = min(ssp_clk, rate);
>  
>  	/*
>  	 * Calculate the divisor for the SCR (Serial Clock Rate), avoiding
> @@ -902,8 +902,7 @@ static unsigned int ssp_get_clk_div(struct driver_data *drv_data, int rate)
>  		return (DIV_ROUND_UP(ssp_clk, rate) - 1)  & 0xfff;
>  }
>  
> -static unsigned int pxa2xx_ssp_get_clk_div(struct driver_data *drv_data,
> -					   int rate)
> +static unsigned int pxa2xx_ssp_get_clk_div(struct driver_data *drv_data, u32 rate)
>  {
>  	struct chip_data *chip =
>  		spi_get_ctldata(drv_data->controller->cur_msg->spi);