From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9D428241E7 for ; Fri, 6 Sep 2024 04:03:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.67.36.66 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725595434; cv=none; b=E1oWesMqB43FFI0jd0e8lGT/hUWuHa4P3NZyGRyZr0JvYNdD/b4MzpMw8mSfIgT6dSKMa9FcTKlrnMlymr6XFzxRxG/+0q9R4A0MruoqakcAETToaLv3TFBHVjHpKWxIuVCTtpYHYEN6dtarQHF0/12NJdq7N9Dzp9JTGV2FGhQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725595434; c=relaxed/simple; bh=Cl+XZOT8hYc3b65NiZ1qivtNXFlq5W1ZUyAoyCh1HDg=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=G2vwQfvJztUgXEmW810fyja1RjXrqL9K1N/ck9waSp7OxP4XzZu1fRIeIHayT63+an3IpnOpmO7dljWQmeSpqdbiuvO+j1Qp4ZgXJfqil/F71uW8R+52w4Y6XfOhOp1O+n8gQDPF99VyWcnSy8oB37J9dNpcDXueszLlwPxsCTk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=posteo.net; spf=pass smtp.mailfrom=posteo.net; dkim=pass (2048-bit key) header.d=posteo.net header.i=@posteo.net header.b=mswdmC2f; arc=none smtp.client-ip=185.67.36.66 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=posteo.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=posteo.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=posteo.net header.i=@posteo.net header.b="mswdmC2f" Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id E214E240105 for ; Fri, 6 Sep 2024 06:03:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1725595423; bh=Cl+XZOT8hYc3b65NiZ1qivtNXFlq5W1ZUyAoyCh1HDg=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Transfer-Encoding:From; b=mswdmC2fmYVjYwiROheRTF3zuvXiwLFHC8Idl/PllEgHkKzORWlHHzcfulipFPIsA 0FPKPsTFQT3r1xTFiaC7JNf9ooEg83iZ9w0nYMK+8pboLO+ZYrtARkG1PUpQ2AeSUQ eKCRP38t4z8g8Lx7lpMw6Pi0PE2Q+UMFPpQ/olIdIpva7SBQYv3K+KVSa0G/cBpyN4 jpvJ+bp9cpo0JK2FsmM6SvZFtS+puRTE1HFczcK99Ax5AiP8tq8FD48FLpIsoGkbgD 5hNpp3ATpURw1tSuzjagsIkQpOLQQjFdxkvdr8eor7RCq/U6o/jR45httH+YmAdA0g y+GBCcYKL4mvA== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4X0MzG6gcxz6tvl; Fri, 6 Sep 2024 06:03:38 +0200 (CEST) Date: Fri, 6 Sep 2024 04:03:38 +0000 From: Wilken Gottwalt To: Li Zetao Cc: , , , , , , , , , , , , , , , , , , Subject: Re: [PATCH -next 15/19] hwmon: (corsair-psu) Use devm_hid_hw_start_and_open in corsairpsu_probe() Message-ID: <20240906060338.0e1aecdc@posteo.net> In-Reply-To: <20240904123607.3407364-16-lizetao1@huawei.com> References: <20240904123607.3407364-1-lizetao1@huawei.com> <20240904123607.3407364-16-lizetao1@huawei.com> Precedence: bulk X-Mailing-List: linux-input@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Wed, 4 Sep 2024 20:36:03 +0800 Li Zetao wrote: > Currently, the corsair-psu module needs to maintain hid resources > by itself. Consider using devm_hid_hw_start_and_open helper to ensure > that hid resources are consistent with the device life cycle, and release > hid resources before device is released. At the same time, it can avoid > the goto-release encoding, drop the fail_and_close and fail_and_stop > lables, and directly return the error code when an error occurs. > > Signed-off-by: Li Zetao > --- > drivers/hwmon/corsair-psu.c | 24 +++++------------------- > 1 file changed, 5 insertions(+), 19 deletions(-) > > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c > index f8f22b8a67cd..b574ec9cd00f 100644 > --- a/drivers/hwmon/corsair-psu.c > +++ b/drivers/hwmon/corsair-psu.c > @@ -787,14 +787,10 @@ static int corsairpsu_probe(struct hid_device *hdev, const struct > hid_device_id if (ret) > return ret; > > - ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW); > + ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW); > if (ret) > return ret; > > - ret = hid_hw_open(hdev); > - if (ret) > - goto fail_and_stop; > - > priv->hdev = hdev; > hid_set_drvdata(hdev, priv); > mutex_init(&priv->lock); > @@ -805,13 +801,13 @@ static int corsairpsu_probe(struct hid_device *hdev, const struct > hid_device_id ret = corsairpsu_init(priv); > if (ret < 0) { > dev_err(&hdev->dev, "unable to initialize device (%d)\n", ret); > - goto fail_and_stop; > + return ret; > } > > ret = corsairpsu_fwinfo(priv); > if (ret < 0) { > dev_err(&hdev->dev, "unable to query firmware (%d)\n", ret); > - goto fail_and_stop; > + return ret; > } > > corsairpsu_get_criticals(priv); > @@ -820,20 +816,12 @@ static int corsairpsu_probe(struct hid_device *hdev, const struct > hid_device_id priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "corsairpsu", priv, > &corsairpsu_chip_info, NULL); > > - if (IS_ERR(priv->hwmon_dev)) { > - ret = PTR_ERR(priv->hwmon_dev); > - goto fail_and_close; > - } > + if (IS_ERR(priv->hwmon_dev)) > + return PTR_ERR(priv->hwmon_dev); > > corsairpsu_debugfs_init(priv); > > return 0; > - > -fail_and_close: > - hid_hw_close(hdev); > -fail_and_stop: > - hid_hw_stop(hdev); > - return ret; > } > > static void corsairpsu_remove(struct hid_device *hdev) > @@ -842,8 +830,6 @@ static void corsairpsu_remove(struct hid_device *hdev) > > debugfs_remove_recursive(priv->debugfs); > hwmon_device_unregister(priv->hwmon_dev); > - hid_hw_close(hdev); > - hid_hw_stop(hdev); > } > > static int corsairpsu_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, So far looks fine to me. Reviewed-by: Wilken Gottwalt greetings, Wilken