« Return to Thread: Some development questions
Re: Some development questions
by Nicolas Boullis
::
Rate this Message:
Hi,
On Tue, Aug 26, 2008 at 09:12:54PM +0200, Christian Gmeiner wrote:
> Hi all,
>
> I have spend some time in the sources of the driver now, that I got a
> good understanding of whats going on.
> I have started to make /usr/src/linux/scripts/checkpatch.pl more
> happier about the current source. I want
> to "fix" most of the style issues which are left. You can have a first
> look at my work here:
> http://freehg.org/u/austriancoder/em8300-cgmeiner/
> Hope such patches will find their way into the main repository :)
Some of them will certainly find their way. ;-)
Thanks a lot.
Some comments anyway:
1) It would be nice if you could start your development branches on top
on the latest changeset from the main repository (that was
9328b32dd801 while you used 8bfd80c74fa2). That should make the
possible future merge easier, although there should be no problem
here.
2) It would be nice if you could make all your changesets "atomic"
(http://freehg.org/u/austriancoder/em8300-cgmeiner/rev/1073fc66a4ea
is not really, although it's not really a problem for this one) and
use the first line of the description as a single-line short
description for the whole change.
3) Here are (roughly) the rules I try to follow to be compatible with a
broad range of kernel versions:
- whenever possible, don't use kernel versions at all (think
CONFIG_*)
- if it is necessary to support several different API, whenever
possible, use the more recent one in the code and use a macro as
a wrapper for older kernels
- when 2 API overlap and must be supported, use the more recent one
as early as possible
http://freehg.org/u/austriancoder/em8300-cgmeiner/rev/8073355cea68
does not follow there rules; I'll rewrite it.
By the way, I don't know what /usr/src/linux/scripts/checkpatch.pl
really is, but I think there's no way the em8300 drivers can be merged
into the kernel until our adv717x and bt865 drivers are merged with the
corresponding drivers in the kernel... :-/
> The next thing on my clean up list is to get rid of these "Register
> access macros". If you look closely
> at the source you will find references to write_register and to writel
> - for writing values into registers.
> After a quick research I found this one:
> http://lwn.net/Articles/102232/ (usable > 2.6.9).
> I hope you share my opinion to do it everywhere in the source the
> same... so I want go for writel.
>
> What is your opinion?
Hmmm... Using 2 different syntaxes to deal with the same register
certainly is very ugly!
But I'd say that "write_register(0x2000, 0x1);" is somewhat easier to
read than "writel(0x0, &em->mem[0x2000]);"...
Cheers,
Nicolas
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel
On Tue, Aug 26, 2008 at 09:12:54PM +0200, Christian Gmeiner wrote:
> Hi all,
>
> I have spend some time in the sources of the driver now, that I got a
> good understanding of whats going on.
> I have started to make /usr/src/linux/scripts/checkpatch.pl more
> happier about the current source. I want
> to "fix" most of the style issues which are left. You can have a first
> look at my work here:
> http://freehg.org/u/austriancoder/em8300-cgmeiner/
> Hope such patches will find their way into the main repository :)
Some of them will certainly find their way. ;-)
Thanks a lot.
Some comments anyway:
1) It would be nice if you could start your development branches on top
on the latest changeset from the main repository (that was
9328b32dd801 while you used 8bfd80c74fa2). That should make the
possible future merge easier, although there should be no problem
here.
2) It would be nice if you could make all your changesets "atomic"
(http://freehg.org/u/austriancoder/em8300-cgmeiner/rev/1073fc66a4ea
is not really, although it's not really a problem for this one) and
use the first line of the description as a single-line short
description for the whole change.
3) Here are (roughly) the rules I try to follow to be compatible with a
broad range of kernel versions:
- whenever possible, don't use kernel versions at all (think
CONFIG_*)
- if it is necessary to support several different API, whenever
possible, use the more recent one in the code and use a macro as
a wrapper for older kernels
- when 2 API overlap and must be supported, use the more recent one
as early as possible
http://freehg.org/u/austriancoder/em8300-cgmeiner/rev/8073355cea68
does not follow there rules; I'll rewrite it.
By the way, I don't know what /usr/src/linux/scripts/checkpatch.pl
really is, but I think there's no way the em8300 drivers can be merged
into the kernel until our adv717x and bt865 drivers are merged with the
corresponding drivers in the kernel... :-/
> The next thing on my clean up list is to get rid of these "Register
> access macros". If you look closely
> at the source you will find references to write_register and to writel
> - for writing values into registers.
> After a quick research I found this one:
> http://lwn.net/Articles/102232/ (usable > 2.6.9).
> I hope you share my opinion to do it everywhere in the source the
> same... so I want go for writel.
>
> What is your opinion?
Hmmm... Using 2 different syntaxes to deal with the same register
certainly is very ugly!
But I'd say that "write_register(0x2000, 0x1);" is somewhat easier to
read than "writel(0x0, &em->mem[0x2000]);"...
Cheers,
Nicolas
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Dxr3-devel mailing list
Dxr3-devel@...
https://lists.sourceforge.net/lists/listinfo/dxr3-devel
« Return to Thread: Some development questions
| Free embeddable forum powered by Nabble | Forum Help |
