From owner-freebsd-wireless@FreeBSD.ORG Wed Jul 31 15:36:42 2013 Return-Path: Delivered-To: freebsd-wireless@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id A34EF6EC; Wed, 31 Jul 2013 15:36:42 +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 EB936285F; Wed, 31 Jul 2013 15:36:41 +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 F10C2C2F; Wed, 31 Jul 2013 17:36:40 +0200 (CEST) From: "Cedric GROSS" To: "'Adrian Chadd'" References: <51f3f0ce.055a420a.2e1e.fffff220SMTPIN_ADDED_BROKEN@mx.google.com> <002d01ce8c46$a13b23d0$e3b16b70$@info> In-Reply-To: Subject: RE: [IWN] Reviw split 2 Date: Wed, 31 Jul 2013 17:36:37 +0200 Message-ID: <002701ce8e03$c033f640$409be2c0$@info> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0028_01CE8E14.83BCC640" X-Mailer: Microsoft Office Outlook 12.0 Thread-Index: Ac6N/9POFFLLrjYzQjuKzsYNvbnwdgAA3uHw Content-Language: fr Cc: freebsd-wireless@freebsd.org 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: Wed, 31 Jul 2013 15:36:42 -0000 This is a multi-part message in MIME format. ------=_NextPart_000_0028_01CE8E14.83BCC640 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable > -----Message d'origine----- > De=A0: adrian.chadd@gmail.com [mailto:adrian.chadd@gmail.com] De la = part > de Adrian Chadd > Envoy=E9=A0: mercredi 31 juillet 2013 17:08 > =C0=A0: Cedric GROSS > Cc=A0: freebsd-wireless@freebsd.org > Objet=A0: Re: [IWN] Reviw split 2 >=20 > Hi, >=20 > There's some more whitespace things to fix in your diff. >=20 > - > ->......>.......>.......bus_dmamap_sync(sc->rxq.data_dmat, data->map, > ->......>.......>....... BUS_DMASYNC_POSTREAD); > ->......>.......>.......DPRINTF(sc, IWN_DEBUG_ANY, > +>......>.......>.......>.......DPRINTF(sc, IWN_DEBUG_ANY, > >......>.......>....... "%s: scanning channel %d status %x\n", > >......>.......>....... __func__, scan->chan, le32toh(scan- > >status)); >=20 > .. notice how you've indented DPRINTF there? You should fix that. :) Fixed. >=20 > +#ifdef>IWN_DEBUG > +#define IWN_DESC(x) case x:>...return #x #define COUNTOF(array) > +(sizeof(array) / sizeof(array[0])) >=20 > There should be a tab between the #define and the thing you're > defining, rather than a space. Done. >=20 > + * This function print firmawre register >=20 > .. typo, that should be "firmware" :) Yep fixed. >=20 > +>......}; > +>......DPRINTF(sc, IWN_DEBUG_REGISTER, > + "CSR values: (2nd byte of IWN_INT_COALESCING is > IWN_INT_PERIODIC)%s", > + "\n"); > +>......for (i =3D 0; i < COUNTOF(csr_tbl); i++){ >=20 > .. there needs to be a tab in front of the two lines after the > DPRINTF(). Well, strictly speaking, there should be a tab (to bring it > to the same indent level) and then four spaces (as it's a continuation > of the line above it.) Fixed >=20 > Now, you're making IWN_DEBUG an option, right? Once you've done this, > I'll go make sure you can put it in the kernel config file as a build > option (and I'll enable it by default on i386/amd64.) Yes, I think should be a good thing. >=20 > Thanks! >=20 >=20 > -adrian ------=_NextPart_000_0028_01CE8E14.83BCC640 Content-Type: application/octet-stream; name="register3.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="register3.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 firmware 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_0028_01CE8E14.83BCC640--