Date: Wed, 7 Apr 2010 18:46:03 +0100 From: Rui Paulo <rpaulo@FreeBSD.org> To: Gavin Atkinson <gavin@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r206358 - in head/sys: conf dev/bwi dev/bwn dev/iwn dev/ral dev/usb/wlan dev/wpi modules/wlan net80211 Message-ID: <C71A07E7-3C4C-4FA4-A3F2-320931F59933@FreeBSD.org> In-Reply-To: <1270662144.74915.108.camel@buffy.york.ac.uk> References: <201004071529.o37FTDi0035619@svn.freebsd.org> <1270662144.74915.108.camel@buffy.york.ac.uk>
next in thread | previous in thread | raw e-mail | index | archive | help
On 7 Apr 2010, at 18:42, Gavin Atkinson wrote: > On Wed, 2010-04-07 at 15:29 +0000, Rui Paulo wrote: >> Author: rpaulo >> Date: Wed Apr 7 15:29:13 2010 >> New Revision: 206358 >> URL: http://svn.freebsd.org/changeset/base/206358 >> >> Log: >> net80211 rate control framework (net80211 ratectl). > > This looks to be great work! Thanks for doing this. > > [...] > >> MFC after: 1 months > > The changes change KBI - I don't think it can be MFC'd as-is? > (especially the sys/net80211/ieee80211_var.h changes) It can be MFC'ed but not as-is. > > Also... > >> @@ -3393,8 +3368,8 @@ _bwi_txeof(struct bwi_softc *sc, uint16_ >> bus_dmamap_unload(sc->sc_buf_dtag, tb->tb_dmap); >> >> ni = tb->tb_ni; >> + vap = ni->ni_vap; >> if (tb->tb_ni != NULL) { >> - struct bwi_node *bn = (struct bwi_node *) tb->tb_ni; > > If (tb->tb_ni) can indeed be NULL here, we'll now panic. If not, there's > no need for the conditional. > >> @@ -921,13 +900,12 @@ rt2661_tx_intr(struct rt2661_softc *sc) >> data->m = NULL; >> ni = data->ni; >> data->ni = NULL; >> + vap = ni->ni_vap; >> >> /* if no frame has been sent, ignore */ >> if (ni == NULL) >> continue; >> >> - rn = RT2661_NODE(ni); >> - > > Same here. Either we'll panic, or the test for (ni == NULL) isn't > needed. Right, this needs to be moved down. > >> @@ -1022,6 +1008,7 @@ rt2560_tx_intr(struct rt2560_softc *sc) >> data->m = NULL; >> ieee80211_free_node(data->ni); >> data->ni = NULL; >> + ni = NULL; >> >> /* descriptor is no longer valid */ >> desc->flags &= ~htole32(RT2560_TX_VALID); > > This seems unnecessary - we never read "ni" again. Either we fall out > of the loop and exit the subroutine, or we go round the loop again and > overwrite it. I'll remove it. > >> Index: head/sys/dev/usb/wlan/if_urtw.c >> =================================================================== >> --- head/sys/dev/usb/wlan/if_urtw.c (revision 206357) >> +++ head/sys/dev/usb/wlan/if_urtw.c (revision 206358) >> @@ -62,6 +62,7 @@ __FBSDID("$FreeBSD$"); >> #include <dev/usb/wlan/if_urtwvar.h> >> >> SYSCTL_NODE(_hw_usb, OID_AUTO, urtw, CTLFLAG_RW, 0, "USB Realtek 8187L"); >> +#define URTW_DEBUG >> #ifdef URTW_DEBUG >> int urtw_debug = 0; >> SYSCTL_INT(_hw_usb_urtw, OID_AUTO, debug, CTLFLAG_RW, &urtw_debug, 0, > > Is this change intentional? No. Regards, -- Rui Paulo
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?C71A07E7-3C4C-4FA4-A3F2-320931F59933>