lkml.org 
[lkml]   [2005]   [Aug]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] IPMI driver update part 1, add per-channel IPMB addresses
Andrew Morton wrote:

>Corey Minyard <minyard@acm.org> 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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-08-04 15:29    [W:0.041 / U:0.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site