Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 29 Jul 2013 12:30:19 +0200
From:      "Cedric GROSS" <cg@cgross.info>
To:        <freebsd-wireless@freebsd.org>
Subject:   RE: [IWN] Reviw split 2
Message-ID:  <002d01ce8c46$a13b23d0$e3b16b70$@info>
In-Reply-To: <CAJ-VmokCVB5kNY44hJLbAfOb1DMSHmJAG3QTUZYhmPL1gHwMwA@mail.gmail.com>
References:  <51f3f0ce.055a420a.2e1e.fffff220SMTPIN_ADDED_BROKEN@mx.google.com> <CAJ-VmokCVB5kNY44hJLbAfOb1DMSHmJAG3QTUZYhmPL1gHwMwA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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 <gross.c@free.fr> 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 <bsd.kmod.mk>=0A=

------=_NextPart_000_002E_01CE8C57.64C3F3D0--




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?002d01ce8c46$a13b23d0$e3b16b70$>