From owner-freebsd-wireless@FreeBSD.ORG Mon Jul 29 10:30:24 2013 Return-Path: Delivered-To: freebsd-wireless@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 9CA776BC for ; Mon, 29 Jul 2013 10:30:24 +0000 (UTC) (envelope-from cg@cgross.info) Received: from alpha.kreiz-it.fr (alpha.kreiz-it.fr [IPv6:2001:41d0:8:dda6::1]) by mx1.freebsd.org (Postfix) with ESMTP id 0A7452C08 for ; Mon, 29 Jul 2013 10:30:24 +0000 (UTC) Received: from DirTech (lnr56-1-82-246-51-185.fbx.proxad.net [82.246.51.185]) by alpha.kreiz-it.fr (Postfix) with ESMTPSA id 90BC1582 for ; Mon, 29 Jul 2013 12:30:22 +0200 (CEST) From: "Cedric GROSS" To: References: <51f3f0ce.055a420a.2e1e.fffff220SMTPIN_ADDED_BROKEN@mx.google.com> In-Reply-To: Subject: RE: [IWN] Reviw split 2 Date: Mon, 29 Jul 2013 12:30:19 +0200 Message-ID: <002d01ce8c46$a13b23d0$e3b16b70$@info> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_002E_01CE8C57.64C3F3D0" X-Mailer: Microsoft Office Outlook 12.0 thread-index: Ac6LyDgG2hgfJsOMQLmkQHUb7ezmygAffZ9Q Content-Language: fr X-BeenThere: freebsd-wireless@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Discussions of 802.11 stack, tools device driver development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Jul 2013 10:30:24 -0000 This is a multi-part message in MIME format. ------=_NextPart_000_002E_01CE8C57.64C3F3D0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable > -----Message d'origine----- > De=A0: owner-freebsd-wireless@freebsd.org [mailto:owner-freebsd- > wireless@freebsd.org] De la part de Adrian Chadd > Envoy=E9=A0: dimanche 28 juillet 2013 21:25 > =C0=A0: Cedric GROSS > Cc=A0: freebsd-wireless@freebsd.org > Objet=A0: Re: [IWN] Reviw split 2 >=20 > Hi! >=20 > Feedback time! >=20 > - DPRINTF(sc, IWN_DEBUG_TRACE, "->%s done\n", __func__); > + DPRINTF(sc, IWN_DEBUG_TRACE, "->%s: end\n",__func__); > + >=20 > .. all that did was delete a space and add a new line. Just add the > space back in there, delete the new line; then that part of the diff > will disappear. Replace also "done" by "end" to be coherent with other trace >=20 > - if (!(sc->sc_flags & IWN_FLAG_HAS_11N)) { > + if (!(sc->sc_flags & IWN_FLAG_HAS_11N)){ >=20 > Again, you've just deleted a space. You don't need to do that. :-) = Just > put the space back in, that part of the diff will disappear. >=20 > - break; > +#endif > + break; >=20 > .. the #endif is fine, but you reformatted the "break" line. tsk. >=20 > There's another break, and another DPRINTF that you did that to. Ok >=20 > Then in the include file: >=20 > -#define IWN_INT 0x008 > +#define IWN_INT 0x008 > #define IWN_INT_MASK 0x00c > -#define IWN_FH_INT 0x010 > -#define IWN_RESET 0x020 >=20 > +#define IWN_FH_INT 0x010 > +#define IWN_GPIO_IN 0x018 /* read external chip > pins */ > +#define IWN_RESET 0x020 > #define IWN_GP_CNTRL 0x024 > -#define IWN_HW_REV 0x028 > -#define IWN_EEPROM 0x02c > +#define IWN_HW_REV 0x028 > +#define IWN_EEPROM 0x02c >=20 > .. most of those are just tab spaces. You can eliminate those, and all > you'd be adding in is the GPIO_IN line. >=20 I like to see align values but anyway. >=20 > Here's my .vimrc file. It includes colour marking for tabs/spaces, > indenting, and lines longer than 78 characters. Thank Bernhard for it. > It's great for inspecting code and diffs for things like whitespace > changes, trailing whitespace at the end of a line, tabs which should = be > spaces (and vice versa), etc. >=20 > adrian@lucy:~]> cat ~/.vimrc > " LCD-friendly! > :set bg=3Ddark >=20 > " Do general syntax highlighting > :syntax enable >=20 > " Highlight tab characters as >. (with . being tab spaces) " Highlight > extra spaces (ie, before the end of line) with "C" > :set list > :set listchars=3Dtab:>.,trail:C >=20 > :highlight NearLength ctermbg=3Dmagenta ctermfg=3Dwhite = guibg=3D#592959 > :highlight OverLength ctermbg=3Dred ctermfg=3Dwhite guibg=3D#592929 " = Match > characters after position 78; set highlight to be magenta " Second > match characters after position 80; set highlight to be red :match > NearLength /\%78v.\+/ :2match OverLength /\%81v.\+/ >=20 > " Enable auto-indenting > :set ai >=20 Thanks :) > On 27 July 2013 09:09, Cedric GROSS wrote: > > Hello, > > > > > > > > A new patch. > > > > > > > > This one add : > > > > - a debugging function printing uCode registery. > > > > - IWN_DEBUG make option > > > > > > > > A review has be done also which can safely undefined IWN_DEBUG. > > Previously do so lead to compile error. > > > > > > > > Cedric > > Joined, corrected patch. Cedric ------=_NextPart_000_002E_01CE8C57.64C3F3D0 Content-Type: application/octet-stream; name="register2.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="register2.patch" Index: sys/dev/iwn/if_iwn.c=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sys/dev/iwn/if_iwn.c (revision 253764)=0A= +++ sys/dev/iwn/if_iwn.c (working copy)=0A= @@ -160,7 +160,9 @@=0A= static int iwn_read_eeprom(struct iwn_softc *,=0A= uint8_t macaddr[IEEE80211_ADDR_LEN]);=0A= static void iwn4965_read_eeprom(struct iwn_softc *);=0A= +#ifdef IWN_DEBUG=0A= static void iwn4965_print_power_group(struct iwn_softc *, int);=0A= +#endif=0A= static void iwn5000_read_eeprom(struct iwn_softc *);=0A= static uint32_t iwn_eeprom_channel_flags(struct iwn_eeprom_chan *);=0A= static void iwn_read_eeprom_band(struct iwn_softc *, int);=0A= @@ -320,9 +322,12 @@=0A= static void iwn_scan_curchan(struct ieee80211_scan_state *, unsigned = long);=0A= static void iwn_scan_mindwell(struct ieee80211_scan_state *);=0A= static void iwn_hw_reset(void *, int);=0A= +#ifdef IWN_DEBUG=0A= +static char *iwn_get_csr_string(int);=0A= +static void iwn_debug_register(struct iwn_softc *);=0A= +#endif=0A= =0A= -#define IWN_DEBUG=0A= -#ifdef IWN_DEBUG=0A= +#ifdef IWN_DEBUG=0A= enum {=0A= IWN_DEBUG_XMIT =3D 0x00000001, /* basic xmit operation */=0A= IWN_DEBUG_RECV =3D 0x00000002, /* basic recv operation */=0A= @@ -339,6 +344,7 @@=0A= IWN_DEBUG_CMD =3D 0x00001000, /* cmd submission */=0A= IWN_DEBUG_TXRATE =3D 0x00002000, /* TX rate debugging */=0A= IWN_DEBUG_PWRSAVE =3D 0x00004000, /* Power save operations */=0A= + IWN_DEBUG_REGISTER =3D 0x20000000, /* print chipset register */=0A= IWN_DEBUG_TRACE =3D 0x40000000, /* Print begin and start driver = function */=0A= IWN_DEBUG_FATAL =3D 0x80000000, /* fatal errors */=0A= IWN_DEBUG_ANY =3D 0xffffffff=0A= @@ -924,6 +930,8 @@=0A= struct ieee80211com *ic;=0A= int qid;=0A= =0A= + DPRINTF(sc, IWN_DEBUG_TRACE, "->%s begin\n", __func__);=0A= +=0A= if (ifp !=3D NULL) {=0A= ic =3D ifp->if_l2com;=0A= =0A= @@ -961,7 +969,7 @@=0A= if (ifp !=3D NULL)=0A= if_free(ifp);=0A= =0A= - DPRINTF(sc, IWN_DEBUG_TRACE, "->%s done\n", __func__);=0A= + DPRINTF(sc, IWN_DEBUG_TRACE, "->%s: end\n", __func__);=0A= IWN_LOCK_DESTROY(sc);=0A= return 0;=0A= }=0A= @@ -3202,8 +3210,6 @@=0A= }=0A= case IWN_STATE_CHANGED:=0A= {=0A= - uint32_t *status =3D (uint32_t *)(desc + 1);=0A= -=0A= /*=0A= * State change allows hardware switch change to be=0A= * noted. However, we handle this in iwn_intr as we=0A= @@ -3211,32 +3217,37 @@=0A= */=0A= bus_dmamap_sync(sc->rxq.data_dmat, data->map,=0A= BUS_DMASYNC_POSTREAD);=0A= +#ifdef IWN_DEBUG=0A= + uint32_t *status =3D (uint32_t *)(desc + 1);=0A= DPRINTF(sc, IWN_DEBUG_INTR, "state changed to %x\n",=0A= le32toh(*status));=0A= +#endif=0A= break;=0A= }=0A= case IWN_START_SCAN:=0A= {=0A= + bus_dmamap_sync(sc->rxq.data_dmat, data->map,=0A= + BUS_DMASYNC_POSTREAD);=0A= +#ifdef IWN_DEBUG=0A= struct iwn_start_scan *scan =3D=0A= (struct iwn_start_scan *)(desc + 1);=0A= -=0A= - bus_dmamap_sync(sc->rxq.data_dmat, data->map,=0A= - BUS_DMASYNC_POSTREAD);=0A= - DPRINTF(sc, IWN_DEBUG_ANY,=0A= + DPRINTF(sc, IWN_DEBUG_ANY,=0A= "%s: scanning channel %d status %x\n",=0A= __func__, scan->chan, le32toh(scan->status));=0A= +#endif=0A= break;=0A= }=0A= case IWN_STOP_SCAN:=0A= {=0A= + bus_dmamap_sync(sc->rxq.data_dmat, data->map,=0A= + BUS_DMASYNC_POSTREAD);=0A= +#ifdef IWN_DEBUG=0A= struct iwn_stop_scan *scan =3D=0A= (struct iwn_stop_scan *)(desc + 1);=0A= -=0A= - bus_dmamap_sync(sc->rxq.data_dmat, data->map,=0A= - BUS_DMASYNC_POSTREAD);=0A= DPRINTF(sc, IWN_DEBUG_STATE,=0A= "scan finished nchan=3D%d status=3D%d chan=3D%d\n",=0A= scan->nchan, scan->status, scan->chan);=0A= +#endif=0A= =0A= IWN_UNLOCK(sc);=0A= ieee80211_scan_next(vap);=0A= @@ -3416,6 +3427,9 @@=0A= if (r1 & (IWN_INT_SW_ERR | IWN_INT_HW_ERR)) {=0A= device_printf(sc->sc_dev, "%s: fatal firmware error\n",=0A= __func__);=0A= +#ifdef IWN_DEBUG=0A= + iwn_debug_register(sc);=0A= +#endif=0A= /* Dump firmware error log and stop. */=0A= iwn_fatal_intr(sc);=0A= ifp->if_flags &=3D ~IFF_UP;=0A= @@ -7467,3 +7481,85 @@=0A= iwn_init(sc);=0A= ieee80211_notify_radio(ic, 1);=0A= }=0A= +#ifdef IWN_DEBUG=0A= +#define IWN_DESC(x) case x: return #x=0A= +#define COUNTOF(array) (sizeof(array) / sizeof(array[0]))=0A= +=0A= +/*=0A= + * Transate CSR code to string=0A= + */=0A= +static char *iwn_get_csr_string(int csr)=0A= +{=0A= + switch (csr) {=0A= + IWN_DESC(IWN_HW_IF_CONFIG);=0A= + IWN_DESC(IWN_INT_COALESCING);=0A= + IWN_DESC(IWN_INT);=0A= + IWN_DESC(IWN_INT_MASK);=0A= + IWN_DESC(IWN_FH_INT);=0A= + IWN_DESC(IWN_GPIO_IN);=0A= + IWN_DESC(IWN_RESET);=0A= + IWN_DESC(IWN_GP_CNTRL);=0A= + IWN_DESC(IWN_HW_REV);=0A= + IWN_DESC(IWN_EEPROM);=0A= + IWN_DESC(IWN_EEPROM_GP);=0A= + IWN_DESC(IWN_OTP_GP);=0A= + IWN_DESC(IWN_GIO);=0A= + IWN_DESC(IWN_GP_UCODE);=0A= + IWN_DESC(IWN_GP_DRIVER);=0A= + IWN_DESC(IWN_UCODE_GP1);=0A= + IWN_DESC(IWN_UCODE_GP2);=0A= + IWN_DESC(IWN_LED);=0A= + IWN_DESC(IWN_DRAM_INT_TBL);=0A= + IWN_DESC(IWN_GIO_CHICKEN);=0A= + IWN_DESC(IWN_ANA_PLL);=0A= + IWN_DESC(IWN_HW_REV_WA);=0A= + IWN_DESC(IWN_DBG_HPET_MEM);=0A= + default:=0A= + return "UNKNOWN CSR";=0A= + }=0A= +}=0A= +=0A= +/*=0A= + * This function print firmawre register=0A= + */=0A= +static void=0A= +iwn_debug_register(struct iwn_softc *sc)=0A= +{=0A= + int i;=0A= + static const uint32_t csr_tbl[] =3D {=0A= + IWN_HW_IF_CONFIG,=0A= + IWN_INT_COALESCING,=0A= + IWN_INT,=0A= + IWN_INT_MASK,=0A= + IWN_FH_INT,=0A= + IWN_GPIO_IN,=0A= + IWN_RESET,=0A= + IWN_GP_CNTRL,=0A= + IWN_HW_REV,=0A= + IWN_EEPROM,=0A= + IWN_EEPROM_GP,=0A= + IWN_OTP_GP,=0A= + IWN_GIO,=0A= + IWN_GP_UCODE,=0A= + IWN_GP_DRIVER,=0A= + IWN_UCODE_GP1,=0A= + IWN_UCODE_GP2,=0A= + IWN_LED,=0A= + IWN_DRAM_INT_TBL,=0A= + IWN_GIO_CHICKEN,=0A= + IWN_ANA_PLL,=0A= + IWN_HW_REV_WA,=0A= + IWN_DBG_HPET_MEM,=0A= + };=0A= + DPRINTF(sc, IWN_DEBUG_REGISTER,=0A= + "CSR values: (2nd byte of IWN_INT_COALESCING is = IWN_INT_PERIODIC)%s",=0A= + "\n");=0A= + for (i =3D 0; i < COUNTOF(csr_tbl); i++){=0A= + DPRINTF(sc, IWN_DEBUG_REGISTER," %10s: 0x%08x ",=0A= + iwn_get_csr_string(csr_tbl[i]), IWN_READ(sc, csr_tbl[i]));=0A= + if ((i+1) % 3 =3D=3D 0)=0A= + DPRINTF(sc, IWN_DEBUG_REGISTER,"%s","\n");=0A= + }=0A= + DPRINTF(sc, IWN_DEBUG_REGISTER,"%s","\n");=0A= +}=0A= +#endif=0A= Index: sys/dev/iwn/if_iwnreg.h=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sys/dev/iwn/if_iwnreg.h (revision 253764)=0A= +++ sys/dev/iwn/if_iwnreg.h (working copy)=0A= @@ -62,6 +62,7 @@=0A= #define IWN_INT 0x008=0A= #define IWN_INT_MASK 0x00c=0A= #define IWN_FH_INT 0x010=0A= +#define IWN_GPIO_IN 0x018 /* read external chip pins */=0A= #define IWN_RESET 0x020=0A= #define IWN_GP_CNTRL 0x024=0A= #define IWN_HW_REV 0x028=0A= @@ -69,8 +70,12 @@=0A= #define IWN_EEPROM_GP 0x030=0A= #define IWN_OTP_GP 0x034=0A= #define IWN_GIO 0x03c=0A= +#define IWN_GP_UCODE 0x048=0A= #define IWN_GP_DRIVER 0x050=0A= +#define IWN_UCODE_GP1 0x054=0A= +#define IWN_UCODE_GP1_SET 0x058=0A= #define IWN_UCODE_GP1_CLR 0x05c=0A= +#define IWN_UCODE_GP2 0x060=0A= #define IWN_LED 0x094=0A= #define IWN_DRAM_INT_TBL 0x0a0=0A= #define IWN_SHADOW_REG_CTRL 0x0a8=0A= @@ -79,6 +84,7 @@=0A= #define IWN_HW_REV_WA 0x22c=0A= #define IWN_DBG_HPET_MEM 0x240=0A= #define IWN_DBG_LINK_PWR_MGMT 0x250=0A= +/* Need nic_lock for use above */=0A= #define IWN_MEM_RADDR 0x40c=0A= #define IWN_MEM_WADDR 0x410=0A= #define IWN_MEM_WDATA 0x418=0A= Index: sys/modules/iwn/Makefile=0A= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A= --- sys/modules/iwn/Makefile (revision 253764)=0A= +++ sys/modules/iwn/Makefile (working copy)=0A= @@ -5,4 +5,12 @@=0A= KMOD =3D if_iwn=0A= SRCS =3D if_iwn.c device_if.h bus_if.h pci_if.h=0A= =0A= +.if !defined(KERNBUILDDIR)=0A= +opt_wlan.h:=0A= + echo "#define IEEE80211_DEBUG 1" > ${.TARGET}=0A= +.endif=0A= +.if IWN_DEBUG=0A= +CFLAGS+=3D-DIWN_DEBUG=0A= +.endif=0A= +=0A= .include =0A= ------=_NextPart_000_002E_01CE8C57.64C3F3D0--