Re: [PATCH 2/3] HID: nvidia-shield: Fix some missing function calls() in the probe error handling path

From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: jikos@kernel.org, benjamin.tissoires@redhat.com,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH 2/3] HID: nvidia-shield: Fix some missing function calls() in the probe error handling path
Date: Sun, 27 Aug 2023 12:42:16 -0700	[thread overview]
Message-ID: <871qfoqnl3.fsf@nvidia.com> (raw)
In-Reply-To: <443aef6a6d90011e8fffcd43e1a88cd9e98594ca.1693070958.git.christophe.jaillet@wanadoo.fr> (Christophe JAILLET's message of "Sat, 26 Aug 2023 19:42:18 +0200")

On Sat, 26 Aug, 2023 19:42:18 +0200 Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
> The commit in Fixes updated the error handling path of
> thunderstrike_create() and the remove function but not the error handling
> path of shield_probe(), should an error occur after a successful
> thunderstrike_create() call.
>
> Add the missing calls.
>
> Fixes: 3ab196f88237 ("HID: nvidia-shield: Add battery support for Thunderstrike")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>  drivers/hid/hid-nvidia-shield.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c
> index 66a7478e2c9d..849b3f8409a0 100644
> --- a/drivers/hid/hid-nvidia-shield.c
> +++ b/drivers/hid/hid-nvidia-shield.c
> @@ -1074,9 +1074,11 @@ static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  err_stop:
>  	hid_hw_stop(hdev);
>  err_haptics:

Functionally, the change looks good to me. Can we update this label from
err_haptics to err_ts_create? Because of the label name, I accidentally
forgot to add other cleanup in this context originally... (Rust borrow
checker please rescue me)

> +	power_supply_unregister(ts->base.battery_dev.psy);
>  	if (ts->haptics_dev)
>  		input_unregister_device(ts->haptics_dev);
>  	led_classdev_unregister(&ts->led_dev);
> +	ida_free(&thunderstrike_ida, ts->id);
>  	return ret;
>  }

--
Thanks,

Rahul Rameshbabu