Re: [PATCH] HP iLO driver

Previous thread: sysdev: fix debugging statements in registration code. by Ben Dooks on Thursday, June 12, 2008 - 11:00 am. (1 message)

Next thread: [RFC PATCH 0/2] Merge HUGETLB_PAGE and HUGETLBFS Kconfig options by Adam Litke on Thursday, June 12, 2008 - 11:49 am. (7 messages)
From: David Altobelli
Date: Thursday, June 12, 2008 - 11:23 am

A driver for the HP iLO/iLO2 management processor, which allows userspace
programs to query the management processor.  Programs can open a channel
to the device (/dev/hpilo_ccbN), and use this to send/receive queries.  
The O_EXCL open flag is used to indicate that a particular channel cannot
be shared between processes.  This driver will replace various packages
HP has shipped, including hprsm and hp-ilo.

Please CC me on any replies, thanks for your time.

Signed-off-by: David Altobelli <david.altobelli@hp.com>
---
 Kconfig  |   13 +
 Makefile |    1 
 hpilo.c  |  696 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hpilo.h  |  218 +++++++++++++++++++
 4 files changed, 928 insertions(+)
diff -urpN linux-2.6.25.orig/drivers/char/hpilo.c linux-2.6.25/drivers/char/hpilo.c
--- linux-2.6.25.orig/drivers/char/hpilo.c	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.25/drivers/char/hpilo.c	2008-06-12 13:05:14.000000000 -0500
@@ -0,0 +1,696 @@
+/*
+ * Driver for HP iLO/iLO2 management processor.
+ *
+ * Copyright (C) 2008 Hewlett-Packard Development Company, L.P.
+ *	David Altobelli <david.altobelli@hp.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/pci.h>
+#include <linux/ioport.h>
+#include <linux/device.h>
+#include <linux/file.h>
+#include <linux/cdev.h>
+#include <linux/spinlock.h>
+#include <linux/delay.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
+#include "hpilo.h"
+
+static struct class *ilo_class;
+static unsigned int ilo_major;
+static char ilo_hwdev[MAX_ILO_DEV];
+
+/*
+ * FIFO queues, shared with hardware.
+ *
+ * If a queue has empty slots, an entry is added to the queue tail,
+ * and that entry is marked as occupied.
+ * Entries can be dequeued from ...
From: H. Peter Anvin
Date: Thursday, June 12, 2008 - 11:37 am

Wouldn't /dev/hpilo/ccbN be a more appropriate namespace?

	-hpa
--

From: Altobelli, David
Date: Thursday, June 12, 2008 - 12:37 pm

Yes, that would be more clear.  The code is currently defined to a maximum of 1 hardware device, but this could change, and the dev path should probably reflect that.

How do you feel about /dev/hpiloX/ccbN, where X is 0, 1, ... Num_Devices-1?
--

From: H. Peter Anvin
Date: Thursday, June 12, 2008 - 2:05 pm

I'd rather see /dev/hpilo/dXcY, the point being to try to declutter the 
/dev root.

	-hpa
--

From: Michael Tokarev
Date: Friday, June 13, 2008 - 1:30 am

For a case when there will be only one, or, in very rare theoretical
case, two devices, why bother in the first place?  A subdirectory
containing only one node is more ugly.. ;)

/mjt
--

From: Altobelli, David
Date: Friday, June 13, 2008 - 6:54 am

My notation might have confused you, but there will be several ccb nodes.
The N will range from 0,...MAX_CCB-1, and MAX_CCB is currently defined as 8.
--

From: Marcel Holtmann
Date: Thursday, June 12, 2008 - 1:13 pm

this is udev's job.

Regards

Marcel


--

From: H. Peter Anvin
Date: Thursday, June 12, 2008 - 2:06 pm

Yes and no.  udev, as it should, defaults to the namespace used by sysfs.

	-hpa
--

From: John Stoffel
Date: Thursday, June 12, 2008 - 11:34 am

Hi,

I'm running a AMD X2 box with 4gb of RAM, Ubuntu 8.04 x86_64.  I just
rebooted the system and I notice the following repeated messages
(errors? warnings?) in my dmesg output.  I think this is from my
nForce ethernet chips, but I'm not sure.

[   13.268005] ck804xrom ck804xrom_init_one(): Unable to register resource 0x00000000ff000000-0x00000000ffffffff - kernel bug?
[   13.336752] i2c-adapter i2c-0: nForce2 SMBus adapter at 0x1c00
[   13.336786] i2c-adapter i2c-1: nForce2 SMBus adapter at 0x1c40
[   13.376771] CFI: Found no ck804xrom @ffc00000 device at location zero
[   13.398810] JEDEC: Found no ck804xrom @ffc00000 device at location zero
[   13.398822] CFI: Found no ck804xrom @ffc00000 device at location zero
[   13.398883] JEDEC: Found no ck804xrom @ffc00000 device at location zero
[   13.398889] CFI: Found no ck804xrom @ffc00000 device at location zero
[   13.398928] JEDEC: Found no ck804xrom @ffc00000 device at location zero
[   13.398934] CFI: Found no ck804xrom @ffc10000 device at location zero
[   13.398971] JEDEC: Found no ck804xrom @ffc10000 device at location zero
[   13.398980] CFI: Found no ck804xrom @ffc10000 device at location zero
[   13.399042] JEDEC: Found no ck804xrom @ffc10000 device at location zero
[   13.399048] CFI: Found no ck804xrom @ffc10000 device at location zero
[   13.399579] JEDEC: Found no ck804xrom @ffc10000 device at location zero
[   13.399579] CFI: Found no ck804xrom @ffc20000 device at location zero
[   13.399582] JEDEC: Found no ck804xrom @ffc20000 device at location zero
[   13.399591] CFI: Found no ck804xrom @ffc20000 device at location zero
[   13.399652] JEDEC: Found no ck804xrom @ffc20000 device at location zero
[   13.399658] CFI: Found no ck804xrom @ffc20000 device at location zero
[   13.399696] JEDEC: Found no ck804xrom @ffc20000 device at location zero
[   13.399702] CFI: Found no ck804xrom @ffc30000 device at location zero
[   13.399739] JEDEC: Found no ck804xrom @ffc30000 device at location zero
[   13.399748] CFI: Found ...
From: Jean Delvare
Date: Thursday, June 12, 2008 - 1:25 pm

Hi John,


Definitely not related to i2c.

Apparently the messages are coming from drivers/mtd/chips/gen_probe.c.
So I doubt it is related to networking either...

They are debugging messages, but printk(KERN_DEBUG ...) is used where
pr_debug(...) should be. The following (untested) patch should fix it:

Subject: [MTD] No debug message when debugging is disabled

Use pr_debug(...) instead of printk(KERN_DEBUG ...) so that the message
is only printed when debugging is enabled.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
 drivers/mtd/chips/gen_probe.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- linux-2.6.26-rc5.orig/drivers/mtd/chips/gen_probe.c	2008-04-17 04:49:44.000000000 +0200
+++ linux-2.6.26-rc5/drivers/mtd/chips/gen_probe.c	2008-06-12 22:20:12.000000000 +0200
@@ -71,8 +71,8 @@ static struct cfi_private *genprobe_iden
 	   interleave and device type, etc. */
 	if (!genprobe_new_chip(map, cp, &cfi)) {
 		/* The probe didn't like it */
-		printk(KERN_DEBUG "%s: Found no %s device at location zero\n",
-		       cp->name, map->name);
+		pr_debug("%s: Found no %s device at location zero\n",
+			 cp->name, map->name);
 		return NULL;
 	}
 


-- 
Jean Delvare
--

From: John Stoffel
Date: Thursday, June 12, 2008 - 7:51 pm

Jean> Definitely not related to i2c.

THanks for the help and quick reply.  I really should have done more
grepping on the source myself.  

Jean> Apparently the messages are coming from
Jean> drivers/mtd/chips/gen_probe.c.  So I doubt it is related to
Jean> networking either...

Heh.  What the heck am I doing with mtd?  *grin*  I must have left it
in when I migrated to my new system here.  

Jean> They are debugging messages, but printk(KERN_DEBUG ...) is used
Jean> where pr_debug(...) should be. The following (untested) patch
Jean> should fix it:

Jean> Subject: [MTD] No debug message when debugging is disabled

Jean> Use pr_debug(...) instead of printk(KERN_DEBUG ...) so that the message
Jean> is only printed when debugging is enabled.

I'm doing a test compile now and a reboot later on this evening and
I'll let you know how it goes.

John


Jean> Signed-off-by: Jean Delvare <khali@linux-fr.org>
Jean> ---
Jean>  drivers/mtd/chips/gen_probe.c |    4 ++--
Jean>  1 file changed, 2 insertions(+), 2 deletions(-)

Jean> --- linux-2.6.26-rc5.orig/drivers/mtd/chips/gen_probe.c	2008-04-17 04:49:44.000000000 +0200
Jean> +++ linux-2.6.26-rc5/drivers/mtd/chips/gen_probe.c	2008-06-12 22:20:12.000000000 +0200
Jean> @@ -71,8 +71,8 @@ static struct cfi_private *genprobe_iden
Jean>  	   interleave and device type, etc. */
Jean>  	if (!genprobe_new_chip(map, cp, &cfi)) {
Jean>  		/* The probe didn't like it */
Jean> -		printk(KERN_DEBUG "%s: Found no %s device at location zero\n",
Jean> -		       cp->name, map->name);
Jean> +		pr_debug("%s: Found no %s device at location zero\n",
Jean> +			 cp->name, map->name);
Jean>  		return NULL;
Jean>  	}
 


Jean> -- 
Jean> Jean Delvare
--

From: John Stoffel
Date: Friday, June 13, 2008 - 12:39 pm

>>>>> "Jean" == Jean Delvare <khali@linux-fr.org> writes:

This patch fixed my issue with excessive warnings on loading various
mtd drivers.  

Tested-by: John stoffel <john@stoffel.org>

Jean> Signed-off-by: Jean Delvare <khali@linux-fr.org>
Jean> ---
Jean>  drivers/mtd/chips/gen_probe.c |    4 ++--
Jean>  1 file changed, 2 insertions(+), 2 deletions(-)

Jean> --- linux-2.6.26-rc5.orig/drivers/mtd/chips/gen_probe.c	2008-04-17 04:49:44.000000000 +0200
Jean> +++ linux-2.6.26-rc5/drivers/mtd/chips/gen_probe.c	2008-06-12 22:20:12.000000000 +0200
Jean> @@ -71,8 +71,8 @@ static struct cfi_private *genprobe_iden
Jean>  	   interleave and device type, etc. */
Jean>  	if (!genprobe_new_chip(map, cp, &cfi)) {
Jean>  		/* The probe didn't like it */
Jean> -		printk(KERN_DEBUG "%s: Found no %s device at location zero\n",
Jean> -		       cp->name, map->name);
Jean> +		pr_debug("%s: Found no %s device at location zero\n",
Jean> +			 cp->name, map->name);
Jean>  		return NULL;
Jean>  	}
 


Jean> -- 
Jean> Jean Delvare
--

From: Heikki Orsila
Date: Thursday, June 12, 2008 - 12:04 pm

Why do you need a volatile? What you probably want is atomic ops. 

Is writing to Q->fifobar (u64 *) endian-safe?

-- 
Heikki Orsila
heikki.orsila@iki.fi
http://www.iki.fi/shd
--

From: Altobelli, David
Date: Thursday, June 12, 2008 - 1:16 pm

This points to a queue that is shared with hardware, and could

No, this is not endian-safe. Good point.  I think converting these
to readl() operations would let me remove the volatile and fix the
endian issue.
--

Previous thread: sysdev: fix debugging statements in registration code. by Ben Dooks on Thursday, June 12, 2008 - 11:00 am. (1 message)

Next thread: [RFC PATCH 0/2] Merge HUGETLB_PAGE and HUGETLBFS Kconfig options by Adam Litke on Thursday, June 12, 2008 - 11:49 am. (7 messages)