public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jgarzik@pobox.com>
To: Jiri Benc <jbenc@suse.cz>
Cc: NetDev <netdev@oss.sgi.com>, LKML <linux-kernel@vger.kernel.org>,
	pavel@suse.cz
Subject: Re: [2/5] ieee80211: ieee80211_device alignment fix and cleanup
Date: Fri, 27 May 2005 23:37:46 -0400	[thread overview]
Message-ID: <4297E78A.4030906@pobox.com> (raw)
In-Reply-To: <20050524151117.5e059d1d@griffin.suse.cz>


Comments on the remaining patches in this series:

1) I am not convinced that the subclassing of net_device should be done 
in this manner.  Read drivers/net/wan/hdlc_generic.c and .../pc300_drv.c 
for examples of how hdlc_device is handled.

Comments and thoughts welcome.


2) Please put all the new, protocol-layer functions 
ieee80211_type_trans(), ieee80211_change_mtu(), ieee80211_header(), etc. 
in their own file.


3) Temporary or not, the following construct is simply not necessary. 
Modify the definition rather than repeating this two-call piece of code 
in multiple areas:

	ieee80211_priv(netdev_priv(dev));


4) A low-level wireless hardware driver should look like other net 
drivers, and use the existing driver API.  The low level driver should 
implement its own dev->hard_start_xmit().

  	ieee = netdev_priv(dev);
-	dev->hard_start_xmit = ieee80211_xmit;
-
  	ieee->dev = dev;
+	ieee->priv = ieee80211_priv(ieee);
+	
+	dev->hard_start_xmit = ieee80211_xmit;

Certainly, the driver will make many calls to generic ieee80211_xxx 
functions to get things done.

I understand this was not your change; you simply moved the 
dev->hard_start_xmit() assignment down.  I just wanted to take this 
opportunity to make a point.


5) The wireless code in -mm all sourced directly from my netdev-2.6.git 
tree.  I strongly encourage everyone who wants to work on wireless to 
download git (read http://lkml.org/lkml/2005/5/26/11) and check out 
branches 'we18' (wireless ex 18), 'we18-ieee80211' (we18 + ieee80211), 
and 'we18-ieee80211-wifi' (we18 + ieee80211 + hostAP) of
rsync://rsync.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git





  reply	other threads:[~2005-05-28  3:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-05-24 13:07 [0/5] Improvements to the ieee80211 layer Jiri Benc
2005-05-24 13:10 ` [1/5] ieee80211: cleanup Jiri Benc
2005-05-28  2:43   ` Jeff Garzik
2005-05-24 13:11 ` [2/5] ieee80211: ieee80211_device alignment fix and cleanup Jiri Benc
2005-05-28  3:37   ` Jeff Garzik [this message]
2005-05-24 13:12 ` [3/5] netdev: HH_DATA_OFF bugfix Jiri Benc
2005-05-28  3:15   ` Jeff Garzik
2005-05-24 13:12 ` [4/5] ieee80211: ethernet independency Jiri Benc
2005-05-24 13:13 ` [5/5] ieee80211: add sequence numbers Jiri Benc
2005-05-24 18:52 ` [0/5] Improvements to the ieee80211 layer Pavel Machek
2005-05-25  6:55 ` Zhu Yi
2005-05-28  3:18 ` Jeff Garzik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4297E78A.4030906@pobox.com \
    --to=jgarzik@pobox.com \
    --cc=jbenc@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@oss.sgi.com \
    --cc=pavel@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox