On Wed, Apr 13, 2005 at 12:22:10AM -0400, Mark M. Hoffman wrote: > Hi Greg, thanks for your advice: > > * Greg KH <greg at kroah.com> [2005-04-12 14:55:55 -0700]: > > On Mon, Apr 11, 2005 at 12:44:27AM -0400, Mark M. Hoffman wrote: > > > Hi Khali, Greg, et. al: > > > > > > After a brief chat w/ Khali last week (and some reading LDDv3) I did these > > > patches. The result for me looks like this: > > > > > > /sys/class/hwmon/1-0290/device/ -> /sys/devices/platform/i2c-1/1-0290 > > > > > > Notes/Questions: > > > > First a question from me, why? What are you trying to do here? Who > > will be using the hwmon class? And what for? Will all "sensor-like" > > sysfs files move to this new class, or is it just to enable userspace to > > be able to find the hwmon devices easier? > > Khali is working on patches to get rid of the i2c-isa 'pseudo-adapter'. > Since any sensor chip attached to the ISA bus will no longer appear in > the /sys/bus/i2c tree, a new way is needed. Userspace tools can be > taught to look in /sys/class/hwmon first, and then /sys/bus/i2c for > backwards-compatibility. Ok, that sounds acceptable. > > > 1. I don't like "1-0290" for a class ID, but it was the easiest thing to > > > use at the time. A better name might be w83627thf-0. > > > > As long as it's unique, that's the only requirement. > > I just strlcpy'd the '1-0290' ID from the /sys/bus/i2c tree because it > is unique. My concern is that it's not very descriptive. True, pick something else if you don't like that :) > > > 2. The kernel complains if I don't provide a class::release function, but > > > I can't really see a need for it in this case. If I hold a file open through > > > the /sys/class/hwmon/X/device/Y link and then try to rmmod the driver, the > > > rmmod will sleep until I close the file just as I expect. Any hints here > > > Greg? > > > > Ick, yes, you _must_ have a release function. _Please_ listen to the > > kernel, it isn't saying those things for no reason... If you add a file > > to the class device, and do not have a release function, then you can > > But... I didn't add any files to the class device. ;) I'm not planning > to move any existing files (temp1_input, etc) - I just want the link > back to the device. It didn't look like that (by itself) should require > a release function, but I'll go back and read it again. Ok, if you have _no_ sysfs files, and you have no where anyone else could ever grab ahold of a reference to you, then no, you don't need a release function. But that's really not how a class_device structure should be used. > > have a memory leak. The way that you use the class device owned by a > > different device is not safe, nor a good idea. I recommend using the > > Err... what about struct i2c_adapter? Yeah, that's a hack, I wouldn't look at how I intregrated the i2c code into the driver core as _any_ model that should be copied :) > > "class_device_create()" function that has been added to the -mm tree (or > > the class_simple stuff that's in mainline, if you want an idea of what > > it is. Hm, no that will not quite work, that's why I wrote the > > class_device_create() call...use that.) > > OK, I'll try it again that way. Thanks, I think you'll find it a lot easier. greg k-h
