Skip to contentSkip to conversation

[Fwd: PATCH: bmcsensors and i2c-ipmi port to 2.6]

PATCH New Yani Ioannou 3 participants 5 messages May 19, 2005

lore
Share & tools
lore

Yani Ioannou

author
#0 lore raw

PATCH 1/1 [Fwd: PATCH: bmcsensors and i2c-ipmi port to 2.6]

Hi, attached is my e-mail to the author of the bmcsensors and i2c-ipmi driver (*Mark D. Studebaker*), I realized I probably should have CCed the original message to the lm-sensors development team as a whole.

Thanks :-),
Yani
-------------- next part --------------
An embedded message was scrubbed...
From: Yani Ioannou <[email protected]>
Subject: PATCH: bmcsensors and i2c-ipmi port to 2.6
Date: Mon, 15 Mar 2004 11:36:32 -0500
Size: 22254
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20040316/fd7de55f/bmcsensorsandi2c-ipmiportto2.mht

#1 lore raw

PATCH 1/1 [Fwd: PATCH: bmcsensors and i2c-ipmi port to 2.6]

Greg,

Quoted 6 lines
Quoted 4 lines · Yani Ioannou

I can't imagine why
the device sysfs callbacks don't pass information on the file they pertain to, like the base sysfs callbacks, I can only think there must be a good reason, or something I'm missing.

See how I am not the only one complaining about the lack of an additional parameter to i2c sysfs callback functions, that would let us have a single callback function for several similar sysfs entries, much like we were doing in 2.4.

Now that Yani seems to pretend other (non-i2c) sysfs callback functions work that way, I am even more confused and really wonder why it was decided we would not be able to use that possibility.

Signature · 2 lines
Jean Delvare
http://www.ensicaen.ismra.fr/~delvare/
#2 lore raw

PATCH 1/1 [Fwd: PATCH: bmcsensors and i2c-ipmi port to 2.6]

In reply to Yani Ioannou

thanks for the submission.
hopefully you can figure out a way to generate the sysfs entries dynamically. The static generation is unfortunate. Perhaps Greg could help us here. Also I got mail from one person who had 70 sensors on his BMC.

I'm not running 2.6 on my BMC machine but I may try to backport the write code and test it on 2.4.

We've recently changed the sysfs naming convention (from temp_min1 to temp1_min, for example), so we'll need the naming reworked.

mds

Quoted 62 lines · Yani Ioannou

Hi, attached is my e-mail to the author of the bmcsensors and i2c-ipmi driver (*Mark D. Studebaker*), I realized I probably should have CCed the original message to the lm-sensors development team as a whole.

Thanks :-),
Yani

------------------------------------------------------------------------

Subject:
PATCH: bmcsensors and i2c-ipmi port to 2.6
From:
Yani Ioannou <[email protected]>
Date:
Mon, 15 Mar 2004 11:36:32 -0500
To:
"M. Studebaker" <[email protected]>

Hi,

As the kernel newbie faq puts it I've been "pissing in your pool" :-P so I thought it best to contact you. I love the 2.6 kernel but I also loved being able to read the sensors off my HP workstation x4000's IPMI BMC, so I took to porting the driver to 2.6/sysfs. I seem to have been fairly successful, although this could definitely do with some more testing on different hardware.

First of all I'd like to thank you immensely for writing the driver :-) I was very pleased beyond words when I could finally use the IPMI BMC on my motherboard to see my sensors readings, although ironically with my motherboard I can't read 13 voltage, fan and temperatures sensors but not the CPU temperatures! I think being one of the first IPMI 1.0 implementations they messed up the smbus, the adm1021 driver doesn't seem to be able to read the Xeon's temps either.

Attached you will find a patch for kernel 2.6.4-rc2. My main problems porting the driver were in the ability of a BMC to have any number of sensors (or up to 50 as set in your driver). The sysfs show and store callbacks (more specifically the device.h implementation of it) do not appear to provide the ability to pass any information other than the device struct (and the buffer to write into), and thus making a single callback that can work for all sensors turns out to be impossible as far as I can see. Unfortunately (just as with the other ported sensor drivers) I had to create static functions for each sensor id, (i.e. 50!). In the other sensor drivers this is usually only a maximum of 8, so it is not as pronounced. It would make sense to me for the show and store callbacks of device.h to include information on the file that the specific call pertains to (just as the base sysfs implementation does).

Note also that I implemented code to allow the setting of the thresholds for the sensors, but on writing it I found that my BMC doesn't support this, hence they have had no testing whatsoever, and obviously need to be tested on the appropriate hardware. Further improvements that I didn't get around to are creating the sysfs entries for hysteresis thresholds, and of course the driver still only works for one BMC.

I'd be very grateful if you could test out the patch, and if possible test the driver more throughly on other IPMI implementations.

Thanks,
Yani

#3 lore raw

PATCH 1/1 [Fwd: PATCH: bmcsensors and i2c-ipmi port to 2.6]

In reply to Yani Ioannou

the sensors mailing list is the one.
Phil please sign Yani up.

Quoted 38 lines · Yani Ioannou

Wow.. and I thought allowing for 50 sensors was over cautious. Yes, dynamic callbacks would be much more scalable (nevermind easier to code, and less of an eye-sore), but we can't bypass the device.h implementation if only for the kobj hiearchy. I can't imagine why the device sysfs callbacks don't pass information on the file they pertain to, like the base sysfs callbacks, I can only think there must be a good reason, or something I'm missing.

I'll get about to changing the naming when I get time, it won't take much to change, on the dynamic generation of sysfs entries however I could do with all the help I can get :-)

I suppose I really should subscribe to the relevant mailing list, but is that the lm_sensors or kernel one?

Thanks,
Yani

Quoted 15 lines · Mark Studebaker

thanks for the submission.
hopefully you can figure out a way to generate the sysfs entries dynamically.
The static generation is unfortunate. Perhaps Greg could help us here. Also I got mail from one person who had 70 sensors on his BMC.

I'm not running 2.6 on my BMC machine but I may try to backport the write code
and test it on 2.4.

We've recently changed the sysfs naming convention (from temp_min1 to temp1_min,
for example), so we'll need the naming reworked.

mds

author
#4 lore raw

PATCH 1/1 [Fwd: PATCH: bmcsensors and i2c-ipmi port to 2.6]

In reply to Yani Ioannou

Yes, just to clarify I was referring to the sysfs_ops as defined in sysfs.h (and described in Documentation/filesystems/sysfs.txt).

struct sysfs_ops {
        ssize_t (*show)(struct kobject *, struct attribute *,char *);
	ssize_t (*store)(struct kobject *,struct attribute *,const char *);
};

However subsystems such as device.h are free to define their own implementations, as also described in Documentation/filesystems/sysfs.txt as an example, device.h defines

	ssize_t (*show)(struct device * dev, char * buf);
	ssize_t (*store)(struct device * dev, const char * buf);

and maps to the sysfs callbacks by the following code:

static ssize_t
dev_attr_show(struct kobject * kobj, struct attribute * attr, char * buf)

{
         struct device_attribute * dev_attr = to_dev_attr(attr);
         struct device * dev = to_dev(kobj);
         ssize_t ret = 0;
 
         if (dev_attr->show)
                 ret = dev_attr->show(dev,buf);
         return ret;
}

So the main problem for people writing sensors is that for some reason in the device.h implementation of the sysfs callbacks the attribute parameter was removed, I can only think there is a good reason for this (does anybody know why?). I suppose one method might be to implement our own set of callbacks for i2c - but because of the kobject hierarchy (as far as I can see - I could be wrong) we need to go through the device implementation to create the proper sysfs directory structure.

Yani

Quoted 26 lines · Jean Delvare

Greg,

Quoted 10 lines
Quoted 6 lines · Yani Ioannou

I can't imagine why
the device sysfs callbacks don't pass information on the file they pertain to, like the base sysfs callbacks, I can only think there must be a good reason, or something I'm missing.

See how I am not the only one complaining about the lack of an additional parameter to i2c sysfs callback functions, that would let us have a single callback function for several similar sysfs entries, much like we were doing in 2.4.

Now that Yani seems to pretend other (non-i2c) sysfs callback functions work that way, I am even more confused and really wonder why it was decided we would not be able to use that possibility.