Andrew Morton wrote:

Corey Minyard <[EMAIL PROTECTED]> wrote:
ipmi-per-channel-slave-address.patch  unknown/unknown (13533 bytes)]

Could you fix up the mimetype, please?  It makes it hard for various email
clients.
Dang, you switch to a new mail client and everything is screwed up.  Sorry.

IPMI allows multiple IPMB channels on a single interface, and
each channel might have a different IPMB address.  However, the
driver has only one IPMB address that it uses for everything.
This patch adds new IOCTLS and a new internal interface for
setting per-channel IPMB addresses and LUNs.  New systems are
coming out with support for multiple IPMB channels, and they
are broken without this patch.

...
+       for (i=0; i<IPMI_MAX_CHANNELS; i++)

Preferred coding style is actually

        for (i = 0; i < IPMI_MAX_CHANNELS; i++)

but we've kinda lost that fight in drivers :(
Ok, I'll see what I can do. It's the wrong way all over the driver right now.

+#define IPMICTL_SET_MY_CHANNEL_ADDRESS_CMD _IOR(IPMI_IOC_MAGIC, 24, struct 
ipmi_channel_lun_address_set)
+#define IPMICTL_GET_MY_CHANNEL_ADDRESS_CMD _IOR(IPMI_IOC_MAGIC, 25, struct 
ipmi_channel_lun_address_set)
+#define IPMICTL_SET_MY_CHANNEL_LUN_CMD    _IOR(IPMI_IOC_MAGIC, 26, struct 
ipmi_channel_lun_address_set)
+#define IPMICTL_GET_MY_CHANNEL_LUN_CMD    _IOR(IPMI_IOC_MAGIC, 27, struct 
ipmi_channel_lun_address_set)

Are these all OK wrt compat handling?
Yes, it is a structure of an unsigned short and an unsigned char, so it should be ok.

        case IPMICTL_SET_MY_ADDRESS_CMD:
        {
                unsigned int val;
...
        case IPMICTL_GET_MY_ADDRESS_CMD:
        {
-               unsigned int val;
+               unsigned int  val;
+               unsigned char rval;
...
        case IPMICTL_GET_MY_LUN_CMD:
        {
-               unsigned int val;
+               unsigned int  val;
+               unsigned char rval;
+
...
+       case IPMICTL_SET_MY_CHANNEL_ADDRESS_CMD:
+       {
+               struct ipmi_channel_lun_address_set val;
...
+       case IPMICTL_GET_MY_CHANNEL_ADDRESS_CMD:
+       {
+               struct ipmi_channel_lun_address_set val;
...
+       case IPMICTL_SET_MY_CHANNEL_LUN_CMD:
+       {
+               struct ipmi_channel_lun_address_set val;
...
+       case IPMICTL_GET_MY_CHANNEL_LUN_CMD:
+       {
+               struct ipmi_channel_lun_address_set val;
...
        case IPMICTL_SET_TIMING_PARMS_CMD:
        {
                struct ipmi_timing_parms parms;


Be aware that this function will use more stack space than it needs to: gcc
will create a separate stack slot for all the above locals.

Hence it would be better to declare them all at the start of the function. Faster, too - less dcache footprint.

Maybe not as nice from a purist point of view, but it does allow you to
lose those braces in the switch statement...
Hmm, I assumed that gcc would optimize and allocate the stack as it needed it without waste. Ok, easy enough to fix.

Thanks,

-Corey
-
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/

Reply via email to