Re: [PATCH v1] net: wwan: t7xx: Add fastboot interface

[Simon Horman <horms@kernel.org>]

On Thu, Dec 21, 2023 at 03:09:51PM +0800, Jinjian Song wrote:
> From: Jinjian Song <jinjian.song@fibocom.com>
> 
> To support cases such as firmware update or core dump, the t7xx
> device is capable of signaling the host that a special port needs
> to be created before the handshake phase.
> 
> Adds the infrastructure required to create the early ports which
> also requires a different configuration of CLDMA queues.
> 
> On early detection of wwan device in fastboot mode, driver sets
> up CLDMA0 HW tx/rx queues for raw data transfer and then create
> fastboot port to user space.
> 
> Signed-off-by: Jinjian Song <jinjian.song@fibocom.com>

...

> @@ -729,8 +792,17 @@ static int t7xx_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	t7xx_pcie_mac_interrupts_dis(t7xx_dev);
>  
> +	ret = sysfs_create_group(&t7xx_dev->pdev->dev.kobj,
> +				 &t7xx_mode_attribute_group);
> +	if (ret) {
> +		t7xx_md_exit(t7xx_dev);
> +		return ret;
> +	}

Hi Jinjian,

Please consider using goto labels for unwind on error in this function.
Something like this (completely untested!):

	if (ret)
		goto err_md_exit;

	...
	if (ret)
		goto err_remove_group;

	...
	return 0;

err_remove_group:
	sysfs_remove_group(&t7xx_dev->pdev->dev.kobj,
			   &t7xx_mode_attribute_group);
err_md_exit:
	t7xx_md_exit(t7xx_dev);
	return ret;

The reason that I as for this more idiomatic form is that,
in my experience, it is less error prone and easier
to maintain.

> +
>  	ret = t7xx_interrupt_init(t7xx_dev);
>  	if (ret) {
> +		sysfs_remove_group(&t7xx_dev->pdev->dev.kobj,
> +				   &t7xx_mode_attribute_group);
>  		t7xx_md_exit(t7xx_dev);
>  		return ret;
>  	}