Xilinx: hwicap driver comments

[Jiri Slaby]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/