implementing ioctl's for device drivers

hy sapna, here is a working ioctl example with copy_to_user():

// our magic number for our IOCTLs #define FBUS_IOC_MAGIC 'L' // the LOWEST nr we define here, 0x20 #define FBUS_IOC_OFFSET 0x20 // the REAL maximum nr we have defined. absolute maximum is 0x3f #define FBUS_IOC_MAXNR 0x21 // here we define our IOCTLs, Numbers Range from 0x20-0x21: #define FBUS_IOCTL_CPLD_W _IOW(FBUS_IOC_MAGIC, 0x20, data_t *) #define FBUS_IOCTL_CPLD_R _IOR(FBUS_IOC_MAGIC, 0x21, data_t *)

#define EPRINT(args...) do { if (PRINTERRORS) { \ printk(KERN_INFO DEVNAME ": " args); \ } \ } while(0);

static int fbus_ioctl(struct inode *ip, struct file *filp, unsigned int cmd, unsigned long data) { // this variable is needed for the IOCTLs which exchange data with the chip fbus_t *bus = filp->private_data;

// plausibilittschecks: if (_IOC_TYPE(cmd) != FBUS_IOC_MAGIC) return -EINVAL; if (_IOC_NR(cmd) < FBUS_IOC_OFFSET) return -EINVAL; if (_IOC_NR(cmd) > FBUS_IOC_MAXNR) return -EINVAL;

switch(cmd) { case FBUS_IOCTL_CPLD_W: { // write data from userspace to here data_t lb; copy_from_user(&lb, (data_t *)data, sizeof(data_t)); /* * now your data is here in the kernelspace variable lb. use it: */ break; } case FBUS_IOCTL_CPLD_R: { // copy data from here to userspace data_t lb; /* * set the values in "lb", then copy it to userspace: */ copy_to_user((data_t *)data, &lb, sizeof(data_t)); break; } default: EPRINT("unknown IOCTL-nr. 0x%04x\n", _IOC_NR(cmd)); return -EINVAL; }

return 0; }

1. You are not checking the ioctl_num. This is OK for experimenting but you should definitely check it later on and do the operation only if it has the expeced value (IOCTL_SET_LENGTH).
2. You should check the result value of get_user(). It returns -EFAULT, if you passed an invalid pointer.
3. Take a look at the get_user() implementation (taken from uaccess.h):

--------- #define get_user(x,ptr) \ ({ int __ret_gu,__val_gu; \ switch(sizeof (*(ptr))) { \ case 1: __get_user_x(1,__ret_gu,__val_gu,ptr); break; \ case 2: __get_user_x(2,__ret_gu,__val_gu,ptr); break; \ case 4: __get_user_x(4,__ret_gu,__val_gu,ptr); break; \ default: __get_user_x(X,__ret_gu,__val_gu,ptr); break; \ } \ (x) = (__typeof__(*(ptr)))__val_gu; \ __ret_gu; \ })

----------

As you can see, get_user() is a macro. The size of the operand to be copied is determined from the sizeof(*ptr). Your pointer ("temp") is declared as a char* (why? you are copying an integer, so it should be an int*), so this sizeof() will evaluate to a 1 and so only the first byte of the parameter will be copied. This is probably not what you wanted.

You are passing an integer (a 32-bit value), and you do this by passing its address (another 32-bit value). This works, but it is kind of copious (would you do it this way if you passed the value to another function rather than to a driver?) Passing the integer the driver directly would have been easier and would avoid all the get_user() business in the driver.

HTH

Rob

oops ! typing errors... temp is typecast as an int pointer not char pointer as typed. also, the message & msg_ptr are not required. they r actually parts of a bigger code.. not reqd now.