Re: PCI driver

[Manu Abraham <manu@linuxtv.org>]

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