[IWN] Reviw split 2
Adrian Chadd
adrian at freebsd.org
Thu Aug 1 21:49:22 UTC 2013
Committed!
I'm about to commit the config options change so you can add IWN_DEBUG
to your kernel config.
Next! Want to send out an updated diff?
-adrian
On 31 July 2013 08:36, Cedric GROSS <cg at cgross.info> wrote:
>> -----Message d'origine-----
>> De : adrian.chadd at gmail.com [mailto:adrian.chadd at gmail.com] De la part
>> de Adrian Chadd
>> Envoyé : mercredi 31 juillet 2013 17:08
>> À : Cedric GROSS
>> Cc : freebsd-wireless at 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
More information about the freebsd-wireless
mailing list