Jiri Slaby wrote:
On 10/10/05, Manu Abraham <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
