Date: Wed, 31 Jul 2013 17:36:37 +0200 From: "Cedric GROSS" <cg@cgross.info> To: "'Adrian Chadd'" <adrian@freebsd.org> Cc: freebsd-wireless@freebsd.org Subject: RE: [IWN] Reviw split 2 Message-ID: <002701ce8e03$c033f640$409be2c0$@info> In-Reply-To: <CAJ-Vmon4hMbgFKaWva3-HhcJv=eUXKwX7s0uPcD9Nu9g86QEbA@mail.gmail.com> References: <51f3f0ce.055a420a.2e1e.fffff220SMTPIN_ADDED_BROKEN@mx.google.com> <CAJ-VmokCVB5kNY44hJLbAfOb1DMSHmJAG3QTUZYhmPL1gHwMwA@mail.gmail.com> <002d01ce8c46$a13b23d0$e3b16b70$@info> <CAJ-Vmon4hMbgFKaWva3-HhcJv=eUXKwX7s0uPcD9Nu9g86QEbA@mail.gmail.com>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
> -----Message d'origine-----
> De : adrian.chadd@gmail.com [mailto:adrian.chadd@gmail.com] De la part
> de Adrian Chadd
> Envoyé : mercredi 31 juillet 2013 17:08
> À : Cedric GROSS
> Cc : freebsd-wireless@freebsd.org
> Objet : Re: [IWN] Reviw split 2
>
> Hi,
>
> There's some more whitespace things to fix in your diff.
>
> -
> ->......>.......>.......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));
>
> .. notice how you've indented DPRINTF there? You should fix that. :)
Fixed.
>
> +#ifdef>IWN_DEBUG
> +#define IWN_DESC(x) case x:>...return #x #define COUNTOF(array)
> +(sizeof(array) / sizeof(array[0]))
>
> There should be a tab between the #define and the thing you're
> defining, rather than a space.
Done.
>
> + * This function print firmawre register
>
> .. typo, that should be "firmware" :)
Yep fixed.
>
> +>......};
> +>......DPRINTF(sc, IWN_DEBUG_REGISTER,
> + "CSR values: (2nd byte of IWN_INT_COALESCING is
> IWN_INT_PERIODIC)%s",
> + "\n");
> +>......for (i = 0; i < COUNTOF(csr_tbl); i++){
>
> .. 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
>
> 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.
>
> Thanks!
>
>
> -adrian
[-- Attachment #2 --]
Index: sys/dev/iwn/if_iwn.c
===================================================================
--- sys/dev/iwn/if_iwn.c (revision 253764)
+++ sys/dev/iwn/if_iwn.c (working copy)
@@ -160,7 +160,9 @@
static int iwn_read_eeprom(struct iwn_softc *,
uint8_t macaddr[IEEE80211_ADDR_LEN]);
static void iwn4965_read_eeprom(struct iwn_softc *);
+#ifdef IWN_DEBUG
static void iwn4965_print_power_group(struct iwn_softc *, int);
+#endif
static void iwn5000_read_eeprom(struct iwn_softc *);
static uint32_t iwn_eeprom_channel_flags(struct iwn_eeprom_chan *);
static void iwn_read_eeprom_band(struct iwn_softc *, int);
@@ -320,9 +322,12 @@
static void iwn_scan_curchan(struct ieee80211_scan_state *, unsigned long);
static void iwn_scan_mindwell(struct ieee80211_scan_state *);
static void iwn_hw_reset(void *, int);
+#ifdef IWN_DEBUG
+static char *iwn_get_csr_string(int);
+static void iwn_debug_register(struct iwn_softc *);
+#endif
-#define IWN_DEBUG
-#ifdef IWN_DEBUG
+#ifdef IWN_DEBUG
enum {
IWN_DEBUG_XMIT = 0x00000001, /* basic xmit operation */
IWN_DEBUG_RECV = 0x00000002, /* basic recv operation */
@@ -339,6 +344,7 @@
IWN_DEBUG_CMD = 0x00001000, /* cmd submission */
IWN_DEBUG_TXRATE = 0x00002000, /* TX rate debugging */
IWN_DEBUG_PWRSAVE = 0x00004000, /* Power save operations */
+ IWN_DEBUG_REGISTER = 0x20000000, /* print chipset register */
IWN_DEBUG_TRACE = 0x40000000, /* Print begin and start driver function */
IWN_DEBUG_FATAL = 0x80000000, /* fatal errors */
IWN_DEBUG_ANY = 0xffffffff
@@ -924,6 +930,8 @@
struct ieee80211com *ic;
int qid;
+ DPRINTF(sc, IWN_DEBUG_TRACE, "->%s begin\n", __func__);
+
if (ifp != NULL) {
ic = ifp->if_l2com;
@@ -961,7 +969,7 @@
if (ifp != NULL)
if_free(ifp);
- DPRINTF(sc, IWN_DEBUG_TRACE, "->%s done\n", __func__);
+ DPRINTF(sc, IWN_DEBUG_TRACE, "->%s: end\n", __func__);
IWN_LOCK_DESTROY(sc);
return 0;
}
@@ -3202,8 +3210,6 @@
}
case IWN_STATE_CHANGED:
{
- uint32_t *status = (uint32_t *)(desc + 1);
-
/*
* State change allows hardware switch change to be
* noted. However, we handle this in iwn_intr as we
@@ -3211,32 +3217,37 @@
*/
bus_dmamap_sync(sc->rxq.data_dmat, data->map,
BUS_DMASYNC_POSTREAD);
+#ifdef IWN_DEBUG
+ uint32_t *status = (uint32_t *)(desc + 1);
DPRINTF(sc, IWN_DEBUG_INTR, "state changed to %x\n",
le32toh(*status));
+#endif
break;
}
case IWN_START_SCAN:
{
+ bus_dmamap_sync(sc->rxq.data_dmat, data->map,
+ BUS_DMASYNC_POSTREAD);
+#ifdef IWN_DEBUG
struct iwn_start_scan *scan =
(struct iwn_start_scan *)(desc + 1);
-
- 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));
+#endif
break;
}
case IWN_STOP_SCAN:
{
+ bus_dmamap_sync(sc->rxq.data_dmat, data->map,
+ BUS_DMASYNC_POSTREAD);
+#ifdef IWN_DEBUG
struct iwn_stop_scan *scan =
(struct iwn_stop_scan *)(desc + 1);
-
- bus_dmamap_sync(sc->rxq.data_dmat, data->map,
- BUS_DMASYNC_POSTREAD);
DPRINTF(sc, IWN_DEBUG_STATE,
"scan finished nchan=%d status=%d chan=%d\n",
scan->nchan, scan->status, scan->chan);
+#endif
IWN_UNLOCK(sc);
ieee80211_scan_next(vap);
@@ -3416,6 +3427,9 @@
if (r1 & (IWN_INT_SW_ERR | IWN_INT_HW_ERR)) {
device_printf(sc->sc_dev, "%s: fatal firmware error\n",
__func__);
+#ifdef IWN_DEBUG
+ iwn_debug_register(sc);
+#endif
/* Dump firmware error log and stop. */
iwn_fatal_intr(sc);
ifp->if_flags &= ~IFF_UP;
@@ -7467,3 +7481,85 @@
iwn_init(sc);
ieee80211_notify_radio(ic, 1);
}
+#ifdef IWN_DEBUG
+#define IWN_DESC(x) case x: return #x
+#define COUNTOF(array) (sizeof(array) / sizeof(array[0]))
+
+/*
+ * Transate CSR code to string
+ */
+static char *iwn_get_csr_string(int csr)
+{
+ switch (csr) {
+ IWN_DESC(IWN_HW_IF_CONFIG);
+ IWN_DESC(IWN_INT_COALESCING);
+ IWN_DESC(IWN_INT);
+ IWN_DESC(IWN_INT_MASK);
+ IWN_DESC(IWN_FH_INT);
+ IWN_DESC(IWN_GPIO_IN);
+ IWN_DESC(IWN_RESET);
+ IWN_DESC(IWN_GP_CNTRL);
+ IWN_DESC(IWN_HW_REV);
+ IWN_DESC(IWN_EEPROM);
+ IWN_DESC(IWN_EEPROM_GP);
+ IWN_DESC(IWN_OTP_GP);
+ IWN_DESC(IWN_GIO);
+ IWN_DESC(IWN_GP_UCODE);
+ IWN_DESC(IWN_GP_DRIVER);
+ IWN_DESC(IWN_UCODE_GP1);
+ IWN_DESC(IWN_UCODE_GP2);
+ IWN_DESC(IWN_LED);
+ IWN_DESC(IWN_DRAM_INT_TBL);
+ IWN_DESC(IWN_GIO_CHICKEN);
+ IWN_DESC(IWN_ANA_PLL);
+ IWN_DESC(IWN_HW_REV_WA);
+ IWN_DESC(IWN_DBG_HPET_MEM);
+ default:
+ return "UNKNOWN CSR";
+ }
+}
+
+/*
+ * This function print firmware register
+ */
+static void
+iwn_debug_register(struct iwn_softc *sc)
+{
+ int i;
+ static const uint32_t csr_tbl[] = {
+ IWN_HW_IF_CONFIG,
+ IWN_INT_COALESCING,
+ IWN_INT,
+ IWN_INT_MASK,
+ IWN_FH_INT,
+ IWN_GPIO_IN,
+ IWN_RESET,
+ IWN_GP_CNTRL,
+ IWN_HW_REV,
+ IWN_EEPROM,
+ IWN_EEPROM_GP,
+ IWN_OTP_GP,
+ IWN_GIO,
+ IWN_GP_UCODE,
+ IWN_GP_DRIVER,
+ IWN_UCODE_GP1,
+ IWN_UCODE_GP2,
+ IWN_LED,
+ IWN_DRAM_INT_TBL,
+ IWN_GIO_CHICKEN,
+ IWN_ANA_PLL,
+ IWN_HW_REV_WA,
+ IWN_DBG_HPET_MEM,
+ };
+ DPRINTF(sc, IWN_DEBUG_REGISTER,
+ "CSR values: (2nd byte of IWN_INT_COALESCING is IWN_INT_PERIODIC)%s",
+ "\n");
+ for (i = 0; i < COUNTOF(csr_tbl); i++){
+ DPRINTF(sc, IWN_DEBUG_REGISTER," %10s: 0x%08x ",
+ iwn_get_csr_string(csr_tbl[i]), IWN_READ(sc, csr_tbl[i]));
+ if ((i+1) % 3 == 0)
+ DPRINTF(sc, IWN_DEBUG_REGISTER,"%s","\n");
+ }
+ DPRINTF(sc, IWN_DEBUG_REGISTER,"%s","\n");
+}
+#endif
Index: sys/dev/iwn/if_iwnreg.h
===================================================================
--- sys/dev/iwn/if_iwnreg.h (revision 253764)
+++ sys/dev/iwn/if_iwnreg.h (working copy)
@@ -62,6 +62,7 @@
#define IWN_INT 0x008
#define IWN_INT_MASK 0x00c
#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
@@ -69,8 +70,12 @@
#define IWN_EEPROM_GP 0x030
#define IWN_OTP_GP 0x034
#define IWN_GIO 0x03c
+#define IWN_GP_UCODE 0x048
#define IWN_GP_DRIVER 0x050
+#define IWN_UCODE_GP1 0x054
+#define IWN_UCODE_GP1_SET 0x058
#define IWN_UCODE_GP1_CLR 0x05c
+#define IWN_UCODE_GP2 0x060
#define IWN_LED 0x094
#define IWN_DRAM_INT_TBL 0x0a0
#define IWN_SHADOW_REG_CTRL 0x0a8
@@ -79,6 +84,7 @@
#define IWN_HW_REV_WA 0x22c
#define IWN_DBG_HPET_MEM 0x240
#define IWN_DBG_LINK_PWR_MGMT 0x250
+/* Need nic_lock for use above */
#define IWN_MEM_RADDR 0x40c
#define IWN_MEM_WADDR 0x410
#define IWN_MEM_WDATA 0x418
Index: sys/modules/iwn/Makefile
===================================================================
--- sys/modules/iwn/Makefile (revision 253764)
+++ sys/modules/iwn/Makefile (working copy)
@@ -5,4 +5,12 @@
KMOD = if_iwn
SRCS = if_iwn.c device_if.h bus_if.h pci_if.h
+.if !defined(KERNBUILDDIR)
+opt_wlan.h:
+ echo "#define IEEE80211_DEBUG 1" > ${.TARGET}
+.endif
+.if IWN_DEBUG
+CFLAGS+=-DIWN_DEBUG
+.endif
+
.include <bsd.kmod.mk>
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?002701ce8e03$c033f640$409be2c0$>