Re: [PATCH v2 2/2] can: tcan4x5x: Add suspend error handling

["Markus Schneider-Pargmann" <msp@baylibre.com>]

[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]

Hi Martin,

On Fri Oct 16, 2026 at 12:04 PM CEST, Martin Hundebøll wrote:
> Hi Markus,
>
> On Fri, 2026-10-16 at 11:57 +0200, Markus Schneider-Pargmann wrote:
>> diff --git a/drivers/net/can/m_can/tcan4x5x-core.c
>> b/drivers/net/can/m_can/tcan4x5x-core.c
>> index a5b8829aa519..b69312413d62 100644
>> --- a/drivers/net/can/m_can/tcan4x5x-core.c
>> +++ b/drivers/net/can/m_can/tcan4x5x-core.c
>> @@ -575,11 +575,20 @@ static int __maybe_unused
>> tcan4x5x_suspend(struct device *dev)
>>  {
>>  	struct m_can_classdev *cdev = dev_get_drvdata(dev);
>>  	struct spi_device *spi = to_spi_device(dev);
>> +	int ret;
>> 
>>  	if (cdev->pm_wake_source)
>>  		enable_irq_wake(spi->irq);
>> 
>> -	return m_can_class_suspend(dev);
>> +	ret = m_can_class_suspend(dev);
>> +	if (ret)
>> +		goto err_disable_irq;
>> +	return 0;
>> +
>> +err_disable_irq:
>> +	if (cdev->pm_wake_source)
>> +		disable_irq_wake(spi->irq);
>> +	return ret;
>
> Wouldn't this be better written in the shorter form:
>
> 	ret = m_can_class_suspend(dev);
> 	if (ret && dev->pm_wake_source)
> 		disable_irq_wake(spi->irq);
>
> 	return ret;
>
> ?

Thanks for your review. I personally prefer the variant with a goto. It
is not as compact but it is extendable if this function needs another
instruction with error handling at some point.

Best
Markus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]