Re: [PATCH v2 06/13] HID: playstation: track devices in list.

From: "Barnabás Pőcze" <pobrn@protonmail.com>
To: Roderick Colenbrander <roderick@gaikai.com>
Cc: Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
	Chris Ye <lzye@google.com>,
	Roderick Colenbrander <roderick.colenbrander@sony.com>
Subject: Re: [PATCH v2 06/13] HID: playstation: track devices in list.
Date: Thu, 07 Jan 2021 20:13:31 +0000	[thread overview]
Message-ID: <WadJh9J7gmMTty7hDHz6YBWUAdpP5rwz5WGzQvxkhfIImQL4JJ1F2coT3zgP0i0bbtdi2oUtMwWG6qF-4m2TrdhEu2iWJMGHDP2WbB2PTEU=@protonmail.com> (raw)
In-Reply-To: <20210102223109.996781-7-roderick@gaikai.com>

Hi

2021. január 2., szombat 23:31 keltezéssel, Roderick Colenbrander írta:

> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
>
> Track devices in a list, so we can detect when a device is connected
> twice when using Bluetooth and USB.
>
> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> [...]
> @@ -635,6 +671,10 @@  static struct ps_device *dualsense_create(struct hid_device *hdev)
>  	}
>  	snprintf(hdev->uniq, sizeof(hdev->uniq), "%pMR", ds->base.mac_address);
>
> +	ret = ps_devices_list_add(ps_dev);
> +	if (ret < 0)

It's a minor thing, and I have already mentioned it in regards to other functions,
but I believe if `ps_devices_list_add()` can only return 0 on success, or an errno,
then `if (ret)` would be better.

> +		return ERR_PTR(ret);
> +
>  	ret = dualsense_get_calibration_data(ds);
>  	if (ret < 0) {
>  		hid_err(hdev, "Failed to get calibration data from DualSense\n");
> @@ -667,6 +707,7 @@  static struct ps_device *dualsense_create(struct hid_device *hdev)
>  	return &ds->base;
>
>  err:
> +	ps_devices_list_remove(ps_dev);
>  	return ERR_PTR(ret);
>  }
> [...]

Regards,
Barnabás Pőcze