From owner-freebsd-wireless@FreeBSD.ORG Wed Jul 31 16:15:29 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 845BA4D7 for ; Wed, 31 Jul 2013 16:15:29 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-we0-x22c.google.com (mail-we0-x22c.google.com [IPv6:2a00:1450:400c:c03::22c]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 176402A12 for ; Wed, 31 Jul 2013 16:15:28 +0000 (UTC) Received: by mail-we0-f172.google.com with SMTP id t61so808062wes.31 for ; Wed, 31 Jul 2013 09:15:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=yhJLENB9bynAHrbrjJqo8NSF9v5ZRgSJmRWSlMZIrLo=; b=CoufzKdEi0kXSzA2VxCmOCxNSrvikehWquFwdUKyXmM4gbXe3XttyCQ7Ax0xGBNEJb HbhkekBIlPnvL734+el7gtfYcz3U+YfaRG3oyUGpp2GZCAW6WFpKQmKCIp/zdRU6PwBF 2libUK7OFiSMIxYyrF/qvybxeu7aw9C9P8lcqSyRWVETC5+qqalTvqPrc63LuYB0m5uU zN+0LepqVX6IuNVftRm6rh/gPsIA+xukSBufc0SYTfh+C+RwsQPtIUf+2KC0OdR3cQKm uRfknohOuzc2c+kTmau9E1TlOApMhh0RRQZfCNGcZIb0X6fXywV5iQmwzikw1Ci79ept OA2g== MIME-Version: 1.0 X-Received: by 10.180.185.148 with SMTP id fc20mr4946335wic.0.1375287327250; Wed, 31 Jul 2013 09:15:27 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.217.94.132 with HTTP; Wed, 31 Jul 2013 09:15:27 -0700 (PDT) In-Reply-To: <002701ce8e03$c033f640$409be2c0$@info> References: <51f3f0ce.055a420a.2e1e.fffff220SMTPIN_ADDED_BROKEN@mx.google.com> <002d01ce8c46$a13b23d0$e3b16b70$@info> <002701ce8e03$c033f640$409be2c0$@info> Date: Wed, 31 Jul 2013 09:15:27 -0700 X-Google-Sender-Auth: -6YfCcrgd4ZXyX7F3GoGkCHGxbQ Message-ID: Subject: Re: [IWN] Reviw split 2 From: Adrian Chadd To: Cedric GROSS Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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 16:15:29 -0000 Looks good! I'll commit this soon. Thanks! -adrian On 31 July 2013 08:36, Cedric GROSS wrote: >> -----Message d'origine----- >> De : adrian.chadd@gmail.com [mailto:adrian.chadd@gmail.com] De la part >> de Adrian Chadd >> Envoy=E9 : mercredi 31 juillet 2013 17:08 >> =C0 : 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 =3D 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