![]() | ||||||||||
Messages in this thread |
Jiri Slaby wrote:
>On 10/10/05, Manu Abraham <manu@linuxtv.org> wrote:
>
>
>>I have fixed most of the stuff, it is partly working, not ready yet as
>>there are some more things to be added to ..
>>I have attached what i was working on.
>>
>>
>I'm not so bad, I help people (but not 10 times with the same point),
>fuck it and forget.
>
>
>
presumption is not always good, and it is very clear. Most of the
flamewars blaze up because of that. Anyway let's not hover on that ..
>Some points to the new driver:
>indentation isn't so good, the code is not readable well (80 chars on
>a line mainly).
>
>
>
I do agree that indentation is not so good, but how would you suggest
the code be wrapped ?
The problem with wrapping is that readability goes down horribly, but
while debugging a driver, this is too painful.
Considering that it has a long way to go still..
>>static irqreturn_t mantis_pci_irq(int irq, void *dev_id, struct pt_regs *regs)
>>{
>> int count = 0, interrupts = 0;
>> u32 stat = 0, mask = 0, temp;
>>
>> struct mantis_pci *mantis;
>>
>> mantis = (struct mantis_pci *) dev_id;
>>
>>
>you don't need to cast from void *
>struct mantis_pci *mantis = dev_id; is enough
>
>
>
Thanks, Ack'd.
>> mantis->pdev = pdev;
>>
>>
>If you work with this out from pci functions, you should call
>pci_get_dev and in exit function pci_dev_put, otherwise you don't need
>it at all.
>
>
>
Well i am using it in mantis_dma.c, pci_alloc/free
You mean rather than saving off the pointer, i do a pci_get_dev()
and later on in the exit routine, i do a pci_dev_put() .. ?
>mantis_dma_init and others could be __devinit too, or not? Try to use
>it as much as possible.
>
>
>
Any thoughts as to why ? I am not saying it is not needed, but i would
like to know what was the idea.
>>static struct pci_driver mantis_pci_driver = {
>> .name = DRIVER_NAME,
>> .id_table = mantis_pci_table,
>> .probe = mantis_pci_probe,
>> .remove = mantis_pci_remove,
>>
>>
>here should be = __devexit_p(...),
>
>
Ack'd, Thanks ..
>>};
>>
>>
>
>
>
>>err0:
>>
>>
>you should change naming of labels such as errfree, where do you do kfree etc.
>
>
>
Ah , okay.
Thanks,
Manu
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
| |||||||||
| Last update: 2005-10-10 15:16 [from the cache] ©2003-2008 | ||||||||||