On Mon, Jul 09, 2012 at 02:18:48AM +0300, Lazaros Koromilas wrote:
> On Sun, Jul 08, 2012 at 01:31:43PM -0400, Kenneth R Westerback wrote:
> > On Sun, Jul 08, 2012 at 07:17:21PM +0200, Stefan Sperling wrote:
> > > On Sun, Jul 08, 2012 at 08:00:28PM +0300, Lazaros Koromilas wrote:
> > > > On Sun, Jul 08, 2012 at 10:59:09AM +0200, Stefan Sperling wrote:
> > > > > The linux driver ("iwlegacy") doesn't run this command in async mode.
> > > > > Is there a reason why you're passing 1 for the last param, i.e. not
> > > > > waiting for a command-complete interrupt when sending
> > > > > WPI_CMD_ASSOCIATE?
> > > >
> > > > Not really, no. Fixed that. I added printing because all sync
> > > > command calls are handled this way, but can be removed if it's
> > > > not acceptable.
> > >
> > > I think that printf() is fine.
> > >
> > > > > You don't need all of if_flags, just the IFF_PROMISC bit.
> > > > > Why not add a new flag to sc->sc_flags and use that instead?
> > > >
> > > > You are right, I originally added the extra sc_if_flags in order to XOR
> > > > with if_flags and detect the promisc status change. Does this logic
> > > > seem simpler/better? Also removed the initialization above.
> > >
> > > I don't like this approach because it is adding a new 32bit flags field
> > > to the softc, all for checking a single bit from this flags field,
> > > while the existing sc_flags field has lots of unused bits.
> > >
> > > The xor is cute but usually we just use & to check for flags.
>
> Saw this when studying other if_ drivers and thought so too.
Unfortunately those are bad examples.
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
