Hi Krzysztof,
...
> Change the type of "sleep" variable to unsigned and ensure it
> does not use "timeout_ms", when it is a negative value in the
> calculations.
ehm... if it does, though, it's a different, bigger problem :-)
> Signed-off-by: Krzysztof Karas <krzysztof.ka...@intel.com>
> ---
> drivers/gpu/drm/i915/gt/selftest_rps.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_rps.c
> b/drivers/gpu/drm/i915/gt/selftest_rps.c
> index 73bc91c6ea07..20ec7c0c94dc 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_rps.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_rps.c
> @@ -152,7 +152,7 @@ static u8 wait_for_freq(struct intel_rps *rps, u8 freq,
> int timeout_ms)
> {
> u8 history[64], i;
> unsigned long end;
> - int sleep;
> + unsigned int sleep;
I'm not a big fan of this change.
> i = 0;
> memset(history, freq, sizeof(history));
> @@ -180,7 +180,7 @@ static u8 wait_for_freq(struct intel_rps *rps, u8 freq,
> int timeout_ms)
How about doing at this point:
if (!timeout_ms)
return act;
Maybe the caller doesn't want to wait (even though, as Jani said
this validation is a bit overengineered, but I would still accept
it).
If you really want, you could add it at the very beginning to
avoid extra operations.
Andi
> usleep_range(sleep, 2 * sleep);
> sleep *= 2;
> - if (sleep > timeout_ms * 20)
> + if (sleep > timeout_ms * 20 && timeout_ms > 0)
> sleep = timeout_ms * 20;
> } while (1);
> }
> --
> 2.34.1
>
> --
> Best Regards,
> Krzysztof
