[IWN] Review

[Adrian Chadd]

Hi,

Ok. I've reviewed this stuff in more depth.

Split-6 is still way too big to commit as one commit.

I was also hoping that we could get the updated hardware support into this
without necessarily adding the PAN support. But, you're the one driving
this, so it's up to you. :)

* There's DPRINTF() and the debug flags that you've moved into if_iwnreg.h,
which is the wrong place for it!
* .. same as iwn_intr_str()
* You still have style(9) issues:
  + the "if" constructs should be if (), not if();
  + you need spaces between things - eg, if (a == b) rather than if(a==b)
or if (a==b);
  + the } else { should be on one line, not separate on multiple lines.

*  What's the story behind sc->ctx? When is it being set/changed?
* And then there's also ivp->ctx; that's the current VAP context, right? If
that's the case ,why are we bothering checking unit number? Why don't we
consistently check the vap context?

So, let's break up split_6 into this:

* create if_iwn_debug.h; put the debug macros, enum and such into that;
* submit a patch _just_ for the debug work, that's easy to do.

* Then, fix up the style(9) issues.
* Then, help me figure out what the story is with sc->ctx so I understand
what's going on there. I'd like to try and remove that if possible.

Thanks,



-adrian



On 28 September 2013 10:23, Cedric GROSS <cg at cgross.info> wrote:

> Hello,
>
>
>
> I'm get some free time. So I setted up my github for split work.
>
>
>
> So on https://github.com/KreizIT/FreeBSD-IWN/
>
>
>
> You will find 2 new branch : split_6 and split_7.
>
>
>
> Split_6 is iwn -HEAD with split 6 applied. Patch for that is also available
> in this branch.
>
> Split_7 is iwn -HEAD with split 6 and new split 7 applied.
>
>
>
> Split_7 start "parameters task". So no massive change except that it's
> manage NIC without BT.
>
>
>
> Regards
>
> Cedric
>
> _______________________________________________
> freebsd-wireless at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-wireless
> To unsubscribe, send any mail to "freebsd-wireless-unsubscribe at freebsd.org
> "
>