[PATCH] drm/rcar-du: fix comment to rcar_du_group_get()

Alexandra Diupina posted 1 patch 2 years, 3 months ago
drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] drm/rcar-du: fix comment to rcar_du_group_get()
Posted by Alexandra Diupina 2 years, 3 months ago
rcar_du_group_get() never returns a negative
error code (always returns 0), so change
the comment about returned value

Fixes: cb2025d2509f ("drm/rcar-du: Introduce CRTCs groups")
Signed-off-by: Alexandra Diupina <adiupina@astralinux.ru>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
index 2ccd2581f544..499d4e56c32d 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
@@ -200,7 +200,7 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
  *
  * This function must be called with the DRM mode_config lock held.
  *
- * Return 0 in case of success or a negative error code otherwise.
+ * Always return 0.
  */
 int rcar_du_group_get(struct rcar_du_group *rgrp)
 {
-- 
2.30.2
Re: [PATCH] drm/rcar-du: fix comment to rcar_du_group_get()
Posted by Geert Uytterhoeven 2 years, 3 months ago
Hi Alexandra,

On Sun, Sep 3, 2023 at 7:10 PM Alexandra Diupina <adiupina@astralinux.ru> wrote:
> rcar_du_group_get() never returns a negative
> error code (always returns 0), so change
> the comment about returned value
>
> Fixes: cb2025d2509f ("drm/rcar-du: Introduce CRTCs groups")
> Signed-off-by: Alexandra Diupina <adiupina@astralinux.ru>

Thanks for your patch!

> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> @@ -200,7 +200,7 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>   *
>   * This function must be called with the DRM mode_config lock held.
>   *
> - * Return 0 in case of success or a negative error code otherwise.
> + * Always return 0.
>   */
>  int rcar_du_group_get(struct rcar_du_group *rgrp)
>  {

This is debatable: future changes may make it possible for the
function to fail.  In addition, the (single) caller does check the
return value.

If we are sure the function can never fail, and everyone agrees, its
return type should be changed to void, and the caller should be updated.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH] drm/rcar-du: fix comment to rcar_du_group_get()
Posted by Kieran Bingham 2 years, 3 months ago
Hi Alexandra

Quoting Alexandra Diupina (2023-09-03 14:37:09)
> rcar_du_group_get() never returns a negative
> error code (always returns 0), so change
> the comment about returned value

If so, then perhaps this may as well become a void return and remove the
return 0.

That could then clean up some redundant error path handling in
drivers/gpu/drm/renesas/rcar-du/rcar_du_crtc.c too ?

Still, this does correct the documentation to match the implementation
as it stands so... for that ...

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

But removing an unused error path seems like a worthy clean up
opportunity too.

> 
> Fixes: cb2025d2509f ("drm/rcar-du: Introduce CRTCs groups")

Hrm ... well the documented behaviour was the same even before this
commit in rcar_du_get(), so perhaps it was documenting the intent... But
it does seem that the return code has been redundant for quite some time
so perhaps it's just not required.


--
Kieran


> Signed-off-by: Alexandra Diupina <adiupina@astralinux.ru>
> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> index 2ccd2581f544..499d4e56c32d 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_group.c
> @@ -200,7 +200,7 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
>   *
>   * This function must be called with the DRM mode_config lock held.
>   *
> - * Return 0 in case of success or a negative error code otherwise.
> + * Always return 0.
>   */
>  int rcar_du_group_get(struct rcar_du_group *rgrp)
>  {
> -- 
> 2.30.2
>