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.
> 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 :(
> +#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?
> 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...
-
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/
