Re: [PATCH v3] i5k_amb: New memory temperature sensor driver

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Mark M. Hoffman
Date: Thursday, October 18, 2007 - 6:15 am

Hi:

Just a couple more things...

* Darrick J. Wong <djwong@us.ibm.com> [2007-10-16 16:06:57 -0700]:

This member is unused.


You're counting set bits the hard way.

		/* ignore the high-order bit, see "Ugly hack" comment above */
		num_ambs += hweight16(data->amb_present[i] & 0x7fff);

You could use ffs() here, but whatever, it's fine that way too.


Much better, thanks.


Awkward: see suggested patch at the bottom of this message.



diff --git a/drivers/hwmon/i5k_amb.c b/drivers/hwmon/i5k_amb.c
index 3bde717..65ecc56 100644
--- a/drivers/hwmon/i5k_amb.c
+++ b/drivers/hwmon/i5k_amb.c
@@ -400,25 +400,12 @@ out:
 	return res;
 }
 
-static int __devinit i5k_channel_probe(struct i5k_amb_data *data, int which)
+static int __devinit i5k_channel_probe(u16 *amb_present, unsigned long dev_id)
 {
 	struct pci_dev *pcidev;
-	unsigned long dev_id;
 	u16 val16;
 	int res = -ENODEV;
 
-	/* Two memory channels per FBD PCI device */
-	switch (which) {
-	case 0:
-		dev_id = PCI_DEVICE_ID_INTEL_5000_FBD0;
-		break;
-	case 1:
-		dev_id = PCI_DEVICE_ID_INTEL_5000_FBD1;
-		break;
-	default:
-		return -EINVAL;
-	}
-
 	/* Copy the DIMM presence map for these two channels */
 	pcidev = pci_get_device(PCI_VENDOR_ID_INTEL, dev_id, NULL);
 	if (!pcidev)
@@ -426,11 +413,11 @@ static int __devinit i5k_channel_probe(struct i5k_amb_data *data, int which)
 
 	if (pci_read_config_word(pcidev, I5K_REG_CHAN0_PRESENCE_ADDR, &val16))
 		goto out;
-	data->amb_present[which * 2] = val16;
+	amb_present[0] = val16;
 
 	if (pci_read_config_word(pcidev, I5K_REG_CHAN1_PRESENCE_ADDR, &val16))
 		goto out;
-	data->amb_present[which * 2 + 1] = val16;
+	amb_present[1] = val16;
 
 	res = 0;
 
@@ -455,12 +442,14 @@ static int __devinit i5k_amb_probe(struct platform_device *pdev)
 		goto err;
 
 	/* Copy the DIMM presence map for the first two channels */
-	res = i5k_channel_probe(data, 0);
+	res = i5k_channel_probe(&data->amb_present[0],
+			PCI_DEVICE_ID_INTEL_5000_FBD0);
 	if (res)
 		goto err;
 
 	/* Copy the DIMM presence map for the optional second two channels */
-	i5k_channel_probe(data, 1);
+	i5k_channel_probe(&data->amb_present[2], 
+			PCI_DEVICE_ID_INTEL_5000_FBD1);
 
 	/* Set up resource regions */
 	reso = request_mem_region(data->amb_base, data->amb_len, DRVNAME);

Thanks & regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com

-
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH v3] i5k_amb: New memory temperature sensor driver, Darrick J. Wong, (Tue Oct 16, 4:06 pm)
Re: [PATCH v3] i5k_amb: New memory temperature sensor driver, Mark M. Hoffman, (Thu Oct 18, 6:15 am)
[PATCH v4] i5k_amb: New memory temperature sensor driver, Darrick J. Wong, (Thu Oct 18, 1:22 pm)
Re: [PATCH v4] i5k_amb: New memory temperature sensor driver, Mark M. Hoffman, (Tue Oct 23, 4:03 am)