[v3] i2c: ocores: add missing unwind goto in `ocores_i2c_probe`

Smatch Warns:
drivers/i2c/busses/i2c-ocores.c:701 ocores_i2c_probe() warn:
missing unwind goto?

The error handling code after the err_clk label should be executed to
release any resources that were allocated for the clock if a negative
error code returned after platform_get_irq_optional.

Fix this by changing the direct return to `goto err_clk`. 

Signed-off-by: Wang Zhang <silver_code@hust.edu.cn>
---
v2->v3: the first patch had issues. Just do a much simpler fix. No need 
to add the check. clk_disable_unprepare() has checks for error pointer 
and NULL already.
v1->v2: change `ocores_i2c_of_probe` to use `devm_clk_get_enabled()`
---
This patch was found by static analyzer.
...
 drivers/i2c/busses/i2c-ocores.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

On Tue, Apr 18, 2023 at 01:38:13PM +0800, Wang Zhang wrote:
> Smatch Warns:
> drivers/i2c/busses/i2c-ocores.c:701 ocores_i2c_probe() warn:
> missing unwind goto?
> 
> The error handling code after the err_clk label should be executed to
> release any resources that were allocated for the clock if a negative
> error code returned after platform_get_irq_optional.
> 
> Fix this by changing the direct return to `goto err_clk`. 
> 
> Signed-off-by: Wang Zhang <silver_code@hust.edu.cn>
> ---
> v2->v3: the first patch had issues. Just do a much simpler fix. No need 
> to add the check. clk_disable_unprepare() has checks for error pointer 
> and NULL already.
> v1->v2: change `ocores_i2c_of_probe` to use `devm_clk_get_enabled()`

I meant send it to the public mailing list.  Don't add the other
comments which are related to discussions on this list.  Just say that
the public 1 was buggy and the this new v2 patch is simpler.

regards,
dan carpenter

On Tue, Apr 18, 2023 at 08:45:20AM +0300, Dan Carpenter wrote:
> On Tue, Apr 18, 2023 at 01:38:13PM +0800, Wang Zhang wrote:
> > Smatch Warns:
> > drivers/i2c/busses/i2c-ocores.c:701 ocores_i2c_probe() warn:
> > missing unwind goto?
> > 
> > The error handling code after the err_clk label should be executed to
> > release any resources that were allocated for the clock if a negative
> > error code returned after platform_get_irq_optional.
> > 
> > Fix this by changing the direct return to `goto err_clk`. 
> > 
> > Signed-off-by: Wang Zhang <silver_code@hust.edu.cn>
> > ---
> > v2->v3: the first patch had issues. Just do a much simpler fix. No need 
> > to add the check. clk_disable_unprepare() has checks for error pointer 
> > and NULL already.
> > v1->v2: change `ocores_i2c_of_probe` to use `devm_clk_get_enabled()`
> 
> I meant send it to the public mailing list.  Don't add the other
> comments which are related to discussions on this list.  Just say that
> the public 1 was buggy and the this new v2 patch is simpler.

Oh, sorry, this was to the public list.  Never mind.  (Technically both
lists are public).

Some of the notes in --- change log are probably confusing to the people
who aren't on the hust-os-kernel-patches@googlegroups.com list...  Maybe
it's only confusing to me...  LOL.  But the patch itself is straight
forward.

regards,
dan carpenter

I'm sorry for sending the error ping message. 
The related patch has fixed the issue by changing the ocores_i2c_of_probe function to use devm_clk_get_optional_enabled(). 
This new patch only requires a minor modification, and there is no need to add a check for clk_disable_unprepare 
as it already has checks for error pointers and NULL values.

Best regards,
Wang Zhang

"Wang Zhang" <silver_code@hust.edu.cn>写道：
> Smatch Warns:
> drivers/i2c/busses/i2c-ocores.c:701 ocores_i2c_probe() warn:
> missing unwind goto?
> 
> The error handling code after the err_clk label should be executed to
> release any resources that were allocated for the clock if a negative
> error code returned after platform_get_irq_optional.
> 
> Fix this by changing the direct return to `goto err_clk`. 
> 
> Signed-off-by: Wang Zhang <silver_code@hust.edu.cn>
> ---
> v2->v3: the first patch had issues. Just do a much simpler fix. No need 
> to add the check. clk_disable_unprepare() has checks for error pointer 
> and NULL already.
> v1->v2: change `ocores_i2c_of_probe` to use `devm_clk_get_enabled()`
> ---
> This patch was found by static analyzer.
> ...
>  drivers/i2c/busses/i2c-ocores.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> index 2e575856c5cd..1df1439218c9 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -700,8 +700,10 @@ static int ocores_i2c_probe(struct platform_device *pdev)
>  	if (irq == -ENXIO) {
>  		ocores_algorithm.master_xfer = ocores_xfer_polling;
>  	} else {
> -		if (irq < 0)
> -			return irq;
> +		if (irq < 0) {
> +			ret = irq;
> +			goto err_clk;
> +		}
>  	}
>  
>  	if (ocores_algorithm.master_xfer != ocores_xfer_polling) {
> -- 
> 2.34.1