Messages in this thread |  | | | Date | Thu, 07 Feb 2008 21:08:50 +0100 | | From | Jiri Slaby <> | | Subject | Xilinx: hwicap driver comments |
| |
Hi,
first of all, I think that the driver should go through lkml before upstream merge or at least be in -mm for a while (I think this used to be a rule some time ago), correct me if I'm wrong, but none of it happened. Few comments I have: - release f_op retval is silently ignored, I guess you will get your device into undefined state when the first function fails (esp. when you interrupt the sem) - semaphores are deprecated - class_device_create is deprecated - module_init/exit functions should be __init, not __devinit/exit (not a bug, it's subset) - this piece: drvdata = kmalloc(sizeof(struct hwicap_drvdata), GFP_KERNEL); if (!drvdata) { dev_err(dev, "Couldn't allocate device private record\n"); return -ENOMEM; } memset((void *)drvdata, 0, sizeof(struct hwicap_drvdata)); kmalloc + memset = kzalloc null probed_devices[id] on that fail path and on failed1 label
- from/to (void *) casts are useless - io resources are at least ulong - don't understand this: memcpy(kbuf, drvdata->read_buffer, bytes_remaining); drvdata->read_buffer_in_use = bytes_remaining; free_page((unsigned long)kbuf); - can this overlap (=>memmove)? memcpy(drvdata->read_buffer + bytes_to_read, drvdata->read_buffer, 4 - bytes_to_read); - is platform probing function race-proof (like pci)? - run sparse on it, you mix __user with non-__user at least
|  |