lkml.org 
[lkml]   [2007]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: Please pull 'libertas' branch of wireless-2.6
From
Date
On Mon, 2007-05-07 at 11:41 +0100, Christoph Hellwig wrote:
> On Sat, Mar 03, 2007 at 05:21:40AM +0000, Christoph Hellwig wrote:
> > Umm, I can't remember the updated driver ever beeig posted for review.
> > And to be honest I'd be surprised if it's in a good shape already.
>
> Of course it's not anywhere near good shape. Almost all items from my
> review were completely ignored, and we have another totoally substandard
> wireless driver with crappy thread handling, a huge number of broken private
> ioctls and partially absymal codingstyle.

- things like 11d.[ch] don't have business of being in a driver,
this should be somewhere in common code.

There is no common code for this. mac80211 will eventually grow a
utility library that libertas will hopefully be able to attach to.

- please get rid of the ENTER/LEAVE macros

People have said that many drivers do this, and it's quite useful for
debugging.

- please get rid of all your private ioctls and iwpriv stuff
(should I add !!!! here)

Any ioctls that have equivalents in WEXT or ethtool can certainly be
removed. Mesh ones like fwt_reset, bt_reset, mesh_get_ttl, etc have no
current equivalents and will need to stick around. I've already removed
all the ones that were relevant for WPA and moved them to standard WEXT
calls.

- most of types.h should not be there but you should be using
the types from include/linux/*80211*

I've already updated libertas-2.6 git with a ton of updates for this.

In any case, lets push off any merge until 2.6.23 so the rest of the
comments can be dealt with:

- the __le* annotation issue you mentioned in the mail is important.
- there shouldn't be a LICENSE file in individual driver directories,
especially if it's just plain old GPLv2.
- please get rid of setting -DFOO flags in the Makefile, just use
these directly as config symbols.
- there shouldn't be a README file in the driver directory, this
should be in Documentation/
- please don't use wlan_* foo types. a) this should be structs, not
typedefs, and b) wlan is an utterly generic name for beeing inside
a driver.
- there seems to be lots of tabs vs spaces messups
- there's an awful lot of headers without clear divided
responsibilities,
there should be only a few ones left (internal interfaces and hw
interface basically)
- having lowercase names for lots of hw commands is a very bad idea
for readability
- etc

Dan

> And I didn't even get a reply to my mail addressing the concerns above.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

-
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/

\
 
 \ /
  Last update: 2007-05-07 14:03    [W:0.061 / U:0.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site