Re: [PATCH 1/2] firmware: exynos-acpm: use ktime APIs for timeout detection

[Tudor Ambarus]

On 3/21/25 4:40 PM, André Draszik wrote:
> acpm_dequeue_by_polling() uses a loop counter and assumes that each
> iteration of the loop takes 20us. It may take longer, though, because
> usleep_range() may sleep a different amount.
> 
> Switch to using ktime_get() / ktime_after() to detect the timeout
> condition more reliably.
> 
> This change also makes the code easier to follow and it allows us to
> adjust the sleep without having to adjust the loop counter exit
> condition. This will come in useful in a follow-up patch that changes
> the delays.
> 
> Signed-off-by: André Draszik <andre.draszik@xxxxxxxxxx>
> ---
>  drivers/firmware/samsung/exynos-acpm.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
> index a85b2dbdd9f0d7b1f327f54a0a283e4f32587a98..d7ed6b77a957af5db5beba7deecce13ac7b30fd2 100644
> --- a/drivers/firmware/samsung/exynos-acpm.c
> +++ b/drivers/firmware/samsung/exynos-acpm.c
> @@ -32,8 +32,7 @@
>  
>  #define ACPM_PROTOCOL_SEQNUM		GENMASK(21, 16)
>  
> -/* The unit of counter is 20 us. 5000 * 20 = 100 ms */
> -#define ACPM_POLL_TIMEOUT		5000
> +#define ACPM_POLL_TIMEOUT_US		(100 * USEC_PER_MSEC)
>  #define ACPM_TX_TIMEOUT_US		500000
>  
>  #define ACPM_GS101_INITDATA_BASE	0xa000
> @@ -284,12 +283,13 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,
>  				   const struct acpm_xfer *xfer)
>  {
>  	struct device *dev = achan->acpm->dev;
> -	unsigned int cnt_20us = 0;
> +	ktime_t timeout;

#include <linux/ktime.h>

>  	u32 seqnum;
>  	int ret;
>  
>  	seqnum = FIELD_GET(ACPM_PROTOCOL_SEQNUM, xfer->txd[0]);
>  
> +	timeout = ktime_add_us(ktime_get(), ACPM_POLL_TIMEOUT_US);
>  	do {
>  		ret = acpm_get_rx(achan, xfer);
>  		if (ret)
> @@ -300,11 +300,10 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,
>  
>  		/* Determined experimentally. */
>  		usleep_range(20, 30);
> -		cnt_20us++;
> -	} while (cnt_20us < ACPM_POLL_TIMEOUT);
> +	} while (!ktime_after(ktime_get(), timeout));

ktime_before()

With these addressed:
Reviewed-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>