Messages in this thread | | | Date | Mon, 10 Oct 2005 19:00:53 +0400 | From | Manu Abraham <> | Subject | Re: PCI driver |
| |
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/
| |