Re: [PATCH 3/3] hwmon/f71882fg: Make the decision wether to register fan attr. per fan

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

 



Hi,

On 09/12/2011 08:47 PM, Guenter Roeck wrote:
On Fri, 2011-09-09 at 06:12 -0400, Hans de Goede wrote:
Before this patch the f71882fg driver completely fails to initialize
on systems which have reserved settings in the pwm enable register, and
it disables all auto pwm sysfs attributes if any fan is controlled by
a digital sensor reading.

This patch changes the fail to initialize into don't register any attributes
for the fan for which there are reserved settings in the pwm enable register
and also makes the not registering of auto pwm sysfs attributes a per fan
thing.

Signed-off-by: Hans de Goede<hdegoede@xxxxxxxxxx>

Nice cleanup. One minor comment:

[ ... ]

  static int __devinit f71882fg_create_fan_sysfs_files(
-	struct platform_device *pdev, int idx, bool pwm_auto_point)
+	struct platform_device *pdev, int idx)
  {
  	struct f71882fg_data *data = platform_get_drvdata(pdev);
  	int err;

+	/* Sanity check the pwm setting */
+	err = 0;
+	switch (data->type) {
+	case f71858fg:
+		if (((data->pwm_enable>>  (idx * 2))&  3) == 3)
+			err = 1;
+		break;
+	case f71862fg:
+		if (((data->pwm_enable>>  (idx * 2))&  1) != 1)
+			err = 1;
+		break;
+	case f8000:
+		if (idx == 2)
+			err = data->pwm_enable&  0x20;
+		break;
+	default:
+		break;
+	}
+	if (err) {
+		dev_err(&pdev->dev,
+			"Invalid (reserved) pwm settings: 0x%02x, "
+			"skipping fan %d\n",
+			(data->pwm_enable>>  (idx * 2))&  3, idx + 1);
+		return 0; /* This is a non fatal condition */
+	}

Since this is no longer a fatal condition, it might make sense to use
dev_warn instead.

Despite being non fatal, this is still very much an error. f71882fg has
been doing these checks for quite some time now, and Daniel's mobo is the
first one to actual trigger them. Who knows this error may even point out
to some BIOS developer that he is doing something wrong (yeah right BIOS
developers testing with Linux, well we can always hope).

iow I would prefer to keep this as is :)

Regards,

Hans

_______________________________________________
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