Re: [PATCH next] drm/vc4: unlock on error in vc4_hvs_get_fifo_frame_count()

[Dave Stevenson]

Hi Dan

Thanks for the patch.

On Thu, 12 Dec 2024 at 11:31, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> Presumably the default path is never used.  However, if it were used for
> some reason then call drm_dev_exit() before returning.

Correct - the default path would mean something badly wrong.
Without it though, if you add an extra enum value it throws a compiler
warning of an unhandled case.

> Fixes: 8f2fc64773be ("drm/vc4: Fix reading of frame count on GEN5 / Pi4")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
>  drivers/gpu/drm/vc4/vc4_hvs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index b42027636c71..4f524ec126e7 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -522,7 +522,7 @@ u8 vc4_hvs_get_fifo_frame_count(struct vc4_hvs *hvs, unsigned int fifo)
>                 break;
>         default:
>                 drm_err(drm, "Unknown VC4 generation: %d", vc4->gen);
> -               return 0;
> +               field = 0;

field is initialised to 0 anyway to handle the cases where fifo is out of range.

Personally I'd like to see a break in there to ensure we don't
accidentally end up with a fall-through situation if another case got
added at the end. I don't know how others feel.

  Dave

>         }
>
>         drm_dev_exit(idx);
> --
> 2.45.2
>