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

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

[Dan Carpenter]

Presumably the default path is never used.  However, if it were used for
some reason then call drm_dev_exit() before returning.

Fixes: 8f2fc64773be ("drm/vc4: Fix reading of frame count on GEN5 / Pi4")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 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;
 	}
 
 	drm_dev_exit(idx);
-- 
2.45.2

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@linaro.org> 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@linaro.org>
> ---
>  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
>

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

[Dan Carpenter]

On Thu, Dec 12, 2024 at 11:54:28AM +0000, Dave Stevenson wrote:
> I don't know how others feel.

These days we have "warning: this statement may fall through
[-Wimplicit-fallthrough=]" which triggers a build failure so fallthrough
bugs are pretty rare.

But I only care about *your* opinion, Dave.  Everyone else can bounce.

I'll send a v2 which adds the break.

regards,
dan carpenter