Messages in this thread | | | Date | Mon, 10 Oct 2005 21:08:16 +0400 | From | Manu Abraham <> | Subject | Re: PCI driver |
| |
Jiri Slaby wrote:
>On 10/10/05, Manu Abraham <manu@linuxtv.org> wrote: > > >>Jiri Slaby wrote: >> >> >>>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 ? >> >> >Divide strings not "text blabalba \ > continue" >but with better "text blablabalasjdl" > "continue" > >
The dprintk() macro (in mantis_common.h ) was looking very badly with wrap, which Andrew also commented on (about the col's) (the macro being the same, eventhough i was using it elsewhere) it being more than 80 cols, but wrapping the macro made it look like hell.
>wrap line, if it is longer than 80 columns, near last comma, |, & and so on >You seem to use some editor with tab to be less than 8 columns and >some headers are indented bad because of it. > >
My editor uses 8 columns only as a tab, Thunderbird does really handle things a bit different though.
>>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.. >> >> >> >>>> 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 >> >> >And it is called only from places, where pdev is known (i.e. in >parameter of function, e.g. mantis_pci_probe). So you don't need it to >store in mantis, but only call mantis_dma_init(mantis, pdev). Read >below. > > >>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() .. ? >> >> >But if you really want it, call pci_get_dev() and store it into mantis >struct. In the _device_ exit routine call the latter. But I think, >that not to store is better, or the best is to call >mantis_dma_init(pdev) and do pci_get_drvdata inside. > >
i think will pass (pdev) it as a function argument. Looks a bit more cleaner But what i fail to understand is , if you can pass it as an argument, why can't you save the pointer in the struct ?
>>>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. >> >> >Kernel frees up the sections that won't be needed anymore. You put the >function in some section by this. > > Ok,
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/
| |