mbox series

[0/2] Qcom Geni exit path cleanups

Message ID 20241210231054.2844202-1-andi.shyti@kernel.org
Headers show
Series Qcom Geni exit path cleanups | expand

Message

Andi Shyti Dec. 10, 2024, 11:10 p.m. UTC 
Hi,

I am submitting two trivial cleanups in this series. The first 
replaces all instances of dev_err with dev_err_probe throughout 
the probe function for consistency. The second improves the error
exit path by introducing a single 'goto' label for better 
maintainability.

Thank you,  
Andi

Andi Shyti (2):
  i2c: qcom-geni: Use dev_err_probe in the probe function
  i2c: qcom-geni: Simplify error handling in probe function

 drivers/i2c/busses/i2c-qcom-geni.c | 53 ++++++++++++++----------------
 1 file changed, 25 insertions(+), 28 deletions(-)

Comments

Mukesh Kumar Savaliya Dec. 11, 2024, 7:06 a.m. UTC | #1 
Thanks Andi !

On 12/11/2024 12:26 PM, Andi Shyti wrote:
> Hi Mukesh,
> 
> On Wed, Dec 11, 2024 at 09:26:53AM +0530, Mukesh Kumar Savaliya wrote:
>> Thanks Andi for this change !
> 
> Thanks for looking into it.
> 
> ...
> 
>>> @@ -944,8 +938,16 @@ static int geni_i2c_probe(struct platform_device *pdev)
>>>    	return 0;
>> return ret here ? yes, we need to initialize ret = 0.
> 
> here? It's returning '0', as it was before. I'm failing to see
> where 'ret' is used uninitialized. What am I missing?
> 
My point is - Except this place, rest of the places we are returning ret 
OR standard error code. If we return as ret with initializing 0 at the 
start of probe function, it would look good. As such No strict requirement.
>>> +err_off:
>> can we rename as err_resources ?
> 
> yes, it's better, as meaning it's more aligned with the other
> labels.
> 
> Thanks,
> Andi
> 
>>> +	geni_se_resources_off(&gi2c->se);
>>> +err_clk:
>>> +	clk_disable_unprepare(gi2c->core_clk);
>>> +
>>> +	return ret;
>>> +
>>>    err_dma:
>>>    	release_gpi_dma(gi2c);
>>> +
>>>    	return ret;
>>>    }
>>
Andi Shyti Dec. 11, 2024, 9:09 a.m. UTC | #2 
> > > > @@ -944,8 +938,16 @@ static int geni_i2c_probe(struct platform_device *pdev)
> > > >    	return 0;
> > > return ret here ? yes, we need to initialize ret = 0.
> > 
> > here? It's returning '0', as it was before. I'm failing to see
> > where 'ret' is used uninitialized. What am I missing?
> > 
> My point is - Except this place, rest of the places we are returning ret OR
> standard error code. If we return as ret with initializing 0 at the start of
> probe function, it would look good. As such No strict requirement.

Ah, I see. Sure, I can do a "return ret" in my v2.

Thanks,
Andi