Re: [PATCH v3] drm/ast: astdp: fix loop timeout check

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 12.08.24 um 10:29 schrieb Dan Carpenter:
> This code has an issue because it loops until "i" is set to UINT_MAX but
> the test for failure assumes that "i" is set to zero.  The result is that
> it will only print an error message if we succeed on the very last try.
> Reformat the loop to count forwards instead of backwards.
>
> Fixes: 2281475168d2 ("drm/ast: astdp: Perform link training during atomic_enable")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks a lot for the fix. I'll merge it during the week if no other 
comments come in.

Best regards
Thomas

> ---
> v3: V2 had the same bug but just without the always true if (i) statement.
>      Remove the final sleep.
> v2: In V1, I introduced a bug where it would msleep(100) after failure
>      and that is a pointless thing to do.  Also change the loop to a for loop.
> ---
>   drivers/gpu/drm/ast/ast_dp.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index 5d07678b502c..ca022c287785 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -146,18 +146,19 @@ void ast_dp_power_on_off(struct drm_device *dev, bool on)
>   void ast_dp_link_training(struct ast_device *ast)
>   {
>   	struct drm_device *dev = &ast->base;
> -	unsigned int i = 10;
> +	int i;
>   
> -	while (i--) {
> -		u8 vgacrdc = ast_get_index_reg(ast, AST_IO_VGACRI, 0xdc);
> +	for (i = 0; i < 10; i++) {
> +		u8 vgacrdc;
>   
> -		if (vgacrdc & AST_IO_VGACRDC_LINK_SUCCESS)
> -			break;
>   		if (i)
>   			msleep(100);
> +
> +		vgacrdc = ast_get_index_reg(ast, AST_IO_VGACRI, 0xdc);
> +		if (vgacrdc & AST_IO_VGACRDC_LINK_SUCCESS)
> +			return;
>   	}
> -	if (!i)
> -		drm_err(dev, "Link training failed\n");
> +	drm_err(dev, "Link training failed\n");
>   }
>   
>   void ast_dp_set_on_off(struct drm_device *dev, bool on)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)