Re: [PATCH 1/5] hwmon: (corsair-cpro) Use devres function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/21/21 10:13 PM, Jackie Liu wrote:


在 2021/12/22 上午10:58, Guenter Roeck 写道:
On 12/21/21 6:01 PM, Jackie Liu wrote:
From: Jackie Liu <liuyun01@xxxxxxxxxx>

Use devm_hwmon_device_register_with_info() and remove hwmon_dev
from ccp_device struct as it is not needed anymore.

Signed-off-by: Jackie Liu <liuyun01@xxxxxxxxxx>
---
  drivers/hwmon/corsair-cpro.c | 15 ++++++---------
  1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c
index fa6aa4fc8b52..f476367ba6cf 100644
--- a/drivers/hwmon/corsair-cpro.c
+++ b/drivers/hwmon/corsair-cpro.c
@@ -76,7 +76,6 @@
  struct ccp_device {
      struct hid_device *hdev;
-    struct device *hwmon_dev;
      struct completion wait_input_report;
      struct mutex mutex; /* whenever buffer is used, lock before send_usb_cmd */
      u8 *buffer;
@@ -486,6 +485,7 @@ static int get_temp_cnct(struct ccp_device *ccp)
  static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
  {
      struct ccp_device *ccp;
+    struct device *hwmon_dev;
      int ret;
      ccp = devm_kzalloc(&hdev->dev, sizeof(*ccp), GFP_KERNEL);
@@ -523,12 +523,12 @@ static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
      ret = get_fan_cnct(ccp);
      if (ret)
          goto out_hw_close;
-    ccp->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "corsaircpro",
-                             ccp, &ccp_chip_info, 0);
-    if (IS_ERR(ccp->hwmon_dev)) {
-        ret = PTR_ERR(ccp->hwmon_dev);
+    hwmon_dev =
+        devm_hwmon_device_register_with_info(&hdev->dev, "corsaircpro",
+                             ccp, &ccp_chip_info, 0);
+    ret = PTR_ERR_OR_ZERO(hwmon_dev);
+    if (ret)
          goto out_hw_close;
-    }
      return 0;
@@ -541,9 +541,6 @@ static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
  static void ccp_remove(struct hid_device *hdev)
  {
-    struct ccp_device *ccp = hid_get_drvdata(hdev);
-
-    hwmon_device_unregister(ccp->hwmon_dev);
      hid_hw_close(hdev);
      hid_hw_stop(hdev);

The point is that the above two functions need to be called _after_ the hwmon device
was removed. This patch changes the order and removes the hwmon device after the hid
functions have been removed.

If you think this is valid you'll need to explain in detail why removal order
does not matter. Otherwise this patch deserves a NACK.

Guenter



Hi Guenter

After adjusting the order here, there will be a small window for sysfs to continue to provide services. However, because hid has been
disconnected, the read and write interfaces will not get the actual data and returned by timeout. IMO this is not a big issue, but it's okay to not change it.


That is not how kernel development works. I just hope you don't introduce such "not a big issue"
problems in other areas of the kernel.

Guenter



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux