'ret' can only be 0 at this point, being preceded by a 'if (ret) return
ret;'. So return 0 for clarity.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
@@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
 
 	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
 
-	return ret;
+	return 0;
 }
 
 static void imx8qxp_ldb_remove(struct platform_device *pdev)

-- 
2.48.1

On 03/07/2025, Luca Ceresoli wrote:
> 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> ret;'. So return 0 for clarity.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to misc/kernel.git (drm-misc-next).
Thanks!

-- 
Regards,
Liu Ying

On 03/07/2025, Luca Ceresoli wrote:
> 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> ret;'. So return 0 for clarity.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
>  
>  	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
>  
> -	return ret;
> +	return 0;

I guess this is not the only place across the kernel tree where this cleanup
could be done.  So, maybe use some tools to cleanup them all?

>  }
>  
>  static void imx8qxp_ldb_remove(struct platform_device *pdev)
> 

-- 
Regards,
Liu Ying

Hello Liu,

On Fri, 7 Mar 2025 14:42:12 +0800
Liu Ying <victor.liu@nxp.com> wrote:

> On 03/07/2025, Luca Ceresoli wrote:
> > 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> > ret;'. So return 0 for clarity.
> > 
> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > ---
> >  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
> > --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
> >  
> >  	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
> >  
> > -	return ret;
> > +	return 0;  
> 
> I guess this is not the only place across the kernel tree where this cleanup
> could be done.  So, maybe use some tools to cleanup them all?

I had stumbled upon this as I was doing some changes to this function,
and needed to understand the code flow. Definitely 'ret 0' would have
made it  immediately clear that all the code between the last 'if (ret)
return ret;' and the final 'return ret' is not allowed to fail.

I think this change would (slightly, but still) improve future readers'
life.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

On 03/07/2025, Luca Ceresoli wrote:
> Hello Liu,
> 
> On Fri, 7 Mar 2025 14:42:12 +0800
> Liu Ying <victor.liu@nxp.com> wrote:
> 
>> On 03/07/2025, Luca Ceresoli wrote:
>>> 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
>>> ret;'. So return 0 for clarity.
>>>
>>> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>>> ---
>>>  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
>>> index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
>>> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
>>> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
>>> @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
>>>  
>>>  	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
>>>  
>>> -	return ret;
>>> +	return 0;  
>>
>> I guess this is not the only place across the kernel tree where this cleanup
>> could be done.  So, maybe use some tools to cleanup them all?
> 
> I had stumbled upon this as I was doing some changes to this function,
> and needed to understand the code flow. Definitely 'ret 0' would have
> made it  immediately clear that all the code between the last 'if (ret)
> return ret;' and the final 'return ret' is not allowed to fail.
> 
> I think this change would (slightly, but still) improve future readers'
> life.

Reviewed-by: Liu Ying <victor.liu@nxp.com>

> 
> Luca
>

On Fri, Mar 07, 2025 at 12:22:17PM +0100, Luca Ceresoli wrote:
> Hello Liu,
>
> On Fri, 7 Mar 2025 14:42:12 +0800
> Liu Ying <victor.liu@nxp.com> wrote:
>
> > On 03/07/2025, Luca Ceresoli wrote:
> > > 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> > > ret;'. So return 0 for clarity.
> > >
> > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > > ---
> > >  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > > index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
> > > --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > > +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> > > @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
> > >
> > >  	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
> > >
> > > -	return ret;
> > > +	return 0;
> >
> > I guess this is not the only place across the kernel tree where this cleanup
> > could be done.  So, maybe use some tools to cleanup them all?
>
> I had stumbled upon this as I was doing some changes to this function,
> and needed to understand the code flow. Definitely 'ret 0' would have
> made it  immediately clear that all the code between the last 'if (ret)
> return ret;' and the final 'return ret' is not allowed to fail.
>
> I think this change would (slightly, but still) improve future readers'
> life.

I think "return ret" at probe already become common sense for developer.
No value to take efforts to clean up this.

Frank

>
> Luca
>
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

On Thu, Mar 06, 2025 at 06:28:40PM +0100, Luca Ceresoli wrote:
> 'ret' can only be 0 at this point, being preceded by a 'if (ret) return
> ret;'. So return 0 for clarity.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> index 7bce2305d676714cdec7ce085cb53b25ce42f8e7..bee1c6002d5f84dc33b6d5dc123726703baa427e 100644
> --- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> +++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb.c
> @@ -665,7 +665,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
>
>  	ldb_add_bridge_helper(ldb, &imx8qxp_ldb_bridge_funcs);
>
> -	return ret;
> +	return 0;

I don't think it is necessary, no difference.

Frank

>  }
>
>  static void imx8qxp_ldb_remove(struct platform_device *pdev)
>
> --
> 2.48.1
>