This is very good. I believe the structure is correct, but I'm not a
sysfs expert.
There are a few things we need to deal with, though.
* There are some significant changes to versioning in the
driver that are in the mm tree right now (you can pull them from
http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.13-rc6/2.6.13-rc6-mm2/broken-out)
* There are some coding style problems. You have places like:
+static void ipmi_bmc_unregister(struct ipmi_si_device *si,
+ struct ipmi_bmc_device *bmc){
The '{' for functions and structures needs to be on it's own line.
Also, several:
+ if(bmc->guid_present){
that need spaces after the 'if' and before the '{'.
The patch also adds some trailing spaces to empty lines, the
following is a telltale sign:
-
+
There was also one place where you added unneeded braces to
a single statement and another where you deleted the empty line
between two functions.
These are all standard kernel coding style rules.
* I'd prefer to store the product id, device id, and manufacturer id
decoded. This makes it easier to handle (no need to use "memcmp"
to compare) and print. The printing of the product id, for instance,
will be rather unnatural in product_id_show().
* guids are not printable strings (see guid_show()).
* The printing of the guid in ipmi_bmc_register is, well, kind of ugly.
It should probably be moved to its own function that lets you pass in
the rest of the string.
-Corey
Yani Ioannou wrote:
Hi Corey,
Here is a patch against 2.6.13 I've been working on (and mentioned to
you at OLS), that adds support for the 2.6 driver model, and sysfs to
the ipmi subsystem. It is loosely inspired by the USB driver model
system. There is a driver struct added for the ipmi_msghandler, and
one for each system interface 'driver' (for each state machine in
ipmi_si). A device struct is created for each system interface in the
system, and for each unique BMC on the system, with a symlink between
each system interface and the bmc it interfaces to. Each bmc has a set
of fundamental device attributes, and each of the drivers has a
version attribute reflecting the driver/state machine versions.
Furthermore each of these devices is registered as a class_device with
the ipmi class (moved to ipmi_msghandler) as bmcx and smix.
This in effect creates the following sysfs interface on a machine with
a single BMC and two interfaces:
/sys/
|-- block
|-- bus
| |-- platform
| | |-- devices
| | | |-- ipmi_bmc.0 -> ../../../devices/platform/ipmi_bmc.0
| | | |-- ipmi_si.0 -> ../../../devices/platform/ipmi_si.0
| | | |-- ipmi_si.1 -> ../../../devices/platform/ipmi_si.1
| | `-- drivers
| | |-- ipmi_bt_sm
| | | |-- bind
| | | |-- ipmi_si.1 -> ../../../../devices/platform/ipmi_si.1
| | | |-- unbind
| | | `-- version
| | |-- ipmi_kcs_sm
| | | |-- bind
| | | |-- ipmi_si.0 -> ../../../../devices/platform/ipmi_si.0
| | | |-- unbind
| | | `-- version
| | |-- ipmi_msghandler
| | | |-- bind
| | | |-- ipmi_bmc.0 -> ../../../../devices/platform/ipmi_bmc.0
| | | |-- unbind
| | | `-- version
|-- class
| |-- ipmi
| | |-- bmc0
| | | `-- device -> ../../../devices/platform/ipmi_bmc.0
| | |-- ipmi0
| | | `-- dev
| | |-- ipmi1
| | | `-- dev
| | |-- smi0
| | | `-- device -> ../../../devices/platform/ipmi_si.0
| | `-- smi1
| | `-- device -> ../../../devices/platform/ipmi_si.1
|-- devices
| |-- platform
| | |-- ipmi_bmc.0
| | | |-- bus -> ../../../bus/platform
| | | |-- device_id
| | | |-- driver -> ../../../bus/platform/drivers/ipmi_msghandler
| | | |-- firmware_revision
| | | |-- ipmi_version
| | | |-- power
| | | | `-- state
| | | |-- product_id
| | | `-- revision
| | |-- ipmi_si.0
| | | |-- bmc -> ../../../devices/platform/ipmi_bmc.0
| | | |-- bus -> ../../../bus/platform
| | | |-- driver -> ../../../bus/platform/drivers/ipmi_kcs_sm
| | | `-- power
| | | `-- state
| | |-- ipmi_si.1
| | | |-- bmc -> ../../../devices/platform/ipmi_bmc.0
| | | |-- bus -> ../../../bus/platform
| | | |-- driver -> ../../../bus/platform/drivers/ipmi_bt_sm
| | | `-- power
| | | `-- state
|-- kernel
|-- module
| |-- ipmi_devintf
| | |-- refcnt
| | `-- sections
| | `-- __param
| |-- ipmi_msghandler
| | |-- refcnt
| | `-- sections
| | |-- __ksymtab
| | `-- __ksymtab_strings
| |-- ipmi_si
| | |-- refcnt
| | `-- sections
| | `-- __param
`-- power
My motiviation for the patch is to have a device struct (bmc) to
register with the new hwmon class for my in-progress hwmon driver
ipmi_sensors. With this patch the driver can create device attributes
under bmc devices representing sensor readings for the
hwmon/lm_sensors interface.
I have tested the patch on two systems of mine with a single bmc, it
should however also work for a machine with multiple bmcs using the
guid/product+device id to differentiate.
Thanks,
Yani
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |