Re: [PATCH 2.6.12-rc6] char: Add Dell Systems Management Base driver

Alexey Dobriyan
2005-06-16T21:14:42Z

On Thursday 16 June 2005 21:30, Doug Warzecha wrote:

> The Dell Systems Management Base driver is a character driver that
> provides support needed by Dell systems management software to manage
> certain Dell systems. The driver implements ioctls for Dell systems
> management software to use to communicate with the driver.

> --- linux-2.6.12-rc6.orig/drivers/char/dcdbas.c
> +++ linux-2.6.12-rc6/drivers/char/dcdbas.c

> +static void tvm_free_suitable(void *ptr)
> +{
> + kfree(ptr);
> +}

Just call kfree directly.

> +static struct file_operations dcdbas_fops = {
> + owner: THIS_MODULE,
> + ioctl: dcdbas_ioctl,
> +};

Use C99 initializers.

.owner = ...
.ioctl = ...

> +static struct notifier_block dcdbas_reboot_nb = {
> + dcdbas_reboot_notify,
> + NULL,
> + 0
> +};

Here too.

> --- linux-2.6.12-rc6.orig/drivers/char/dcdbas.h
> +++ linux-2.6.12-rc6/drivers/char/dcdbas.h

> +#define ESM_HOLD_OS_ON_SHUTDOWN (41)
> +#define ESM_CANCEL_HOLD_OS_ON_SHUTDOWN (42)
> +#define ESM_TVM_HC_ACTION (43)
> +#define ESM_TVM_ALLOC_MEM (44)
> +#define ESM_CALLINTF_REQ (47)
> +#define ESM_TVM_READ_MEM (48)
> +#define ESM_TVM_WRITE_MEM (49)

> +#define ESM_STATUS_CMD_SUCCESS (0)
> +#define ESM_STATUS_CMD_NOT_IMPLEMENTED (1)
> +#define ESM_STATUS_CMD_BAD (2)
> +#define ESM_STATUS_CMD_TIMEOUT (3)
> +#define ESM_STATUS_CMD_NO_SUCH_DEVICE (7)
> +#define ESM_STATUS_CMD_DEVICE_BAD (9)

> +#define HC_ACTION_NONE (0)

> +#define TVM_SMITYPE_NONE (0)
> +#define TVM_SMITYPE_TYPE1 (1)
> +#define TVM_SMITYPE_TYPE2 (2)
> +#define TVM_SMITYPE_TYPE3 (3)

> +#define ESM_APM_CMD (0x0A0)
> +#define ESM_APM_CMD_HEADER_SIZE (4)
> +#define ESM_APM_POWER_CYCLE (0x10)

> +#define CMOS_BASE_PORT (0x070)
> +#define CMOS_PAGE1_INDEX_PORT (0)
> +#define CMOS_PAGE1_DATA_PORT (1)
> +#define CMOS_PAGE2_INDEX_PORT_PIIX4 (2)
> +#define CMOS_PAGE2_DATA_PORT_PIIX4 (3)
> +#define PE1400_APM_CONTROL_PORT (0x0B0)
> +#define PCAT_APM_CONTROL_PORT (0x0B2)
> +#define PCAT_APM_STATUS_PORT (0x0B3)
> +#define PE1300_CMOS_CMD_STRUCT_PTR (0x38)
> +#define PE1400_CMOS_CMD_STRUCT_PTR (0x70)

> +#define MAX_SYSMGMT_SHORTCMD_PARMBUF_LEN (14)
> +#define MAX_SYSMGMT_LONGCMD_SGENTRY_NUM (16)

> +#define TIMEOUT_USEC_SHORT_SEMA_BLOCKING (10000)
> +#define EXPIRED_TIMER (0)

> +#define TVM_PHYS_MAX (0xffffffffUL)

Useless brackets.

> +struct apm_cmd_t {

Please, drop this "_t". You're not typedefing.

> + u8 command;
> + s8 status;
> + u16 reserved;
> + union __attribute__ ((packed)) {
> + struct __attribute__ ((packed)) {
> + u8 parm[MAX_SYSMGMT_SHORTCMD_PARMBUF_LEN];
> + } shortreq;
> +
> + struct __attribute__ ((packed)) {
> + u16 num_sg_entries;
> + struct __attribute__ ((packed)) {
> + u32 size;
> + u64 addr;
> + } sglist[MAX_SYSMGMT_LONGCMD_SGENTRY_NUM];
> + } longreq;
> + } parameters;
> +} __attribute__ ((packed));

> --- linux-2.6.12-rc6.orig/drivers/char/Kconfig
> +++ linux-2.6.12-rc6/drivers/char/Kconfig

> +config DCDBAS
> + tristate "Dell Systems Management Base driver"
> + default m
> + help
> + The Dell Systems Management Base driver provides support
> + needed by Dell systems management software to manage
> + certain Dell systems.
> +
> + Say Y or M here if you plan to use Dell systems management
> + software to manage your system. If you say M here, the
> + driver will be compiled as a module called dcdbas.

Uhh... 5 deriatives from "manage" and you still haven't described what the
driver is actually needed for. What "certain" systems?