Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 15 Apr 2017 18:23:15 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        rgrimes@freebsd.org
Cc:        Navdeep Parhar <np@freebsd.org>, src-committers@freebsd.org,  svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r316936 - head/sys/dev/cxgbe/iw_cxgbe
Message-ID:  <20170415181258.E1854@besplex.bde.org>
In-Reply-To: <201704150727.v3F7Rt9e010658@pdx.rh.CN85.dnsmgr.net>
References:  <201704150727.v3F7Rt9e010658@pdx.rh.CN85.dnsmgr.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 15 Apr 2017, Rodney W. Grimes wrote:

>> On Fri, Apr 14, 2017 at 06:15:10PM -0700, Rodney W. Grimes wrote:
>>> [ Charset UTF-8 unsupported, converting... ]
>>>> Author: np
>>>> Date: Fri Apr 14 19:15:31 2017
>>>> New Revision: 316936
>>>> URL: https://svnweb.freebsd.org/changeset/base/316936
>>>>
>>>> Log:
>>>>   cxgbe/iw_cxgbe: hw supports 64K (not 32K) Protection Domains.
>>>>
>>>>   MFC after:	3 days
>>>>   Sponsored by:	Chelsio Communications
>>>>
>>>> Modified:
>>>>   head/sys/dev/cxgbe/iw_cxgbe/t4.h
>>>>
>>>> Modified: head/sys/dev/cxgbe/iw_cxgbe/t4.h
>>>> ==============================================================================
>>>> --- head/sys/dev/cxgbe/iw_cxgbe/t4.h	Fri Apr 14 18:56:00 2017	(r316935)
>>>> +++ head/sys/dev/cxgbe/iw_cxgbe/t4.h	Fri Apr 14 19:15:31 2017	(r316936)
>>>> @@ -61,7 +61,7 @@
>>>>
>>>>  #define T4_MAX_NUM_QP (1<<16)
>>>>  #define T4_MAX_NUM_CQ (1<<15)
>>>> -#define T4_MAX_NUM_PD (1<<15)
>>>> +#define T4_MAX_NUM_PD 65536
>>>
>>> Why the change in methods of expressing powers of 2 here?
>>> This, imho, would better match the near by code as
>>> #define	T4_MAX_NUM_PD (1<<16)
>>
>> r316940 removed both of the 1<<foo lines above MAX_NUM_PD and all macros
>> in this part of the header match nearby code:
>>
>> #define T4_MAX_NUM_PD 65536
>> #define T4_MAX_EQ_SIZE 65520
>> #define T4_MAX_IQ_SIZE 65520
>> #define T4_MAX_RQ_SIZE(n) (8192 - (n) - 1)
>                                        ^^^ unexplained off by 1 Magic???
>
> Ok, but why not code those as (1<<16), ((1<<16)-16)  to me it makes
> it more clear as to what is going on here.  I suspect the -16 is
> actually sizeof(something).  So even more clear is
> to do something like:
> #define X ((1<<16)-sizeof(somethingXreleated))
>
> Magic constants like 65520 that dont even have a comment as to
> why they are 65520 are in bad form as well.

When the expressions are even slightly complicated, it can be better
to have magic numbers (but spell ones near 64K in hex).  Try vmparam.h
and related files to figure out where basic addresses are.  Most
are perfectly unreadably parametrized by going through about 5 layers
of nested macros leading back to not very many basic parameters.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170415181258.E1854>