Re: [PATCH 2/3] hwmon: (coretemp) some cleanup work

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

 



于 7/19/2010 11:20 AM, Guenter Roeck 写道:
On Sun, Jul 18, 2010 at 10:50:33PM -0400, Chen Gong wrote:
cleanup some redundant codes in coretemp.c.

Signed-off-by: Chen Gong<gong.chen@xxxxxxxxxxxxxxx>
---
  drivers/hwmon/coretemp.c |   16 +++-------------
  1 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 9232c4e..423ece5 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -54,12 +54,12 @@ struct coretemp_data {
  	const char *name;
  	u32 id;
  	u16 core_id;
+	u8 alarm;
  	char valid;		/* zero until following fields are valid */
  	unsigned long last_updated;	/* in jiffies */
  	int temp;
  	int tjmax;
  	int ttarget;
-	u8 alarm;
  };

  /*
@@ -308,7 +308,7 @@ static int __devinit coretemp_probe(struct platform_device *pdev)
  #ifdef CONFIG_SMP
  	data->core_id = c->cpu_core_id;
  #endif
-	data->name = "coretemp";
+	data->name = DRVNAME;
  	mutex_init(&data->update_lock);

  	/* test if we can access the THERM_STATUS MSR */
@@ -543,8 +543,7 @@ static int __init coretemp_init(void)
  		if (c->cpuid_level>= 6&&  (cpuid_eax(0x06)&  0x01)) {
  			err = coretemp_device_add(i);
  			if (err)
-				goto exit_devices_unreg;
-
+				goto exit;

This error exit including cleanup should be retained.
Failure reasons are memory errors and similar severe problems,
so it makes sense to clean up and bail out.

I don't think so. There are 2 reasons:
1. function coretemp_device_add has dealed with memory failure and similar errors. Afterwards the logic is a little bit unnecessary.

2. why when one device fails, all devices should be removed ? Does this
make sense ? If my consumption is OK, the reason 1 is meaningful.


Also, since this patch changes driver operation, calling it "cleanup"
isn't really appropriate.

If above explanation is reasonable, the title should be changed, or
it is still a cleanup patch. ( Only one line changed :-) )


  		} else {
  			printk(KERN_INFO DRVNAME ": CPU (model=0x%x)"
  				" has no thermal sensor.\n", c->x86_model);
@@ -555,15 +554,6 @@ static int __init coretemp_init(void)
  	register_hotcpu_notifier(&coretemp_cpu_notifier);
  #endif
  	return 0;
-
-exit_devices_unreg:
-	mutex_lock(&pdev_list_mutex);
-	list_for_each_entry_safe(p, n,&pdev_list, list) {
-		platform_device_unregister(p->pdev);
-		list_del(&p->list);
-		kfree(p);
-	}
-	mutex_unlock(&pdev_list_mutex);
  exit:
  	return err;
  }
--
1.7.2.rc3



_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors



[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux