2014-07-05 04:32:14

by Nicholas Krause

[permalink] [raw]
Subject: [PATCH] tile: Add underscores to defintions

In file drv_xgbe_impl.h I fixed the definitions for Size Small,
Size Large, Size Jumbo to have underscores before and after to
follow kernel coding style for internel defentions.

Signed-off-by: Nicholas Krause <[email protected]>
---
arch/tile/include/hv/drv_xgbe_impl.h | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/arch/tile/include/hv/drv_xgbe_impl.h b/arch/tile/include/hv/drv_xgbe_impl.h
index 3a73b2b..51959da 100644
--- a/arch/tile/include/hv/drv_xgbe_impl.h
+++ b/arch/tile/include/hv/drv_xgbe_impl.h
@@ -40,18 +40,9 @@
/** Total available words in the eDMA command FIFO. */
#define EDMA_WDS_TOTAL 128

-
-/*
- * FIXME: These definitions are internal and should have underscores!
- * NOTE: The actual numeric values here are intentional and allow us to
- * optimize the concept "if small ... else if large ... else ...", by
- * checking for the low bit being set, and then for non-zero.
- * These are used as array indices, so they must have the values (0, 1, 2)
- * in some order.
- */
-#define SIZE_SMALL (1) /**< Small packet queue. */
-#define SIZE_LARGE (2) /**< Large packet queue. */
-#define SIZE_JUMBO (0) /**< Jumbo packet queue. */
+#define __SIZE_SMALL__ (1) /**< Small packet queue. */
+#define __SIZE_LARGE__ (2) /**< Large packet queue. */
+#define __SIZE_JUMBO__ (0) /**< Jumbo packet queue. */

/** The number of "SIZE_xxx" values. */
#define NETIO_NUM_SIZES 3
--
1.9.1


2014-07-10 18:20:03

by Chris Metcalf

[permalink] [raw]
Subject: Re: [PATCH] tile: Add underscores to defintions

On 7/5/2014 12:32 AM, Nicholas Krause wrote:
> In file drv_xgbe_impl.h I fixed the definitions for Size Small,
> Size Large, Size Jumbo to have underscores before and after to
> follow kernel coding style for internel defentions.
>
> Signed-off-by: Nicholas Krause <[email protected]>
> ---
> arch/tile/include/hv/drv_xgbe_impl.h | 15 +++------------

I don't think there is a kernel coding style for internal definitions.
It's true that double-underscores mean "more raw" in many places, like
for things like set_bit() vs __set_bit(), but that's not really
applicable here. These files are not exported outside of the kernel
in any case so we don't need any special treatment for them to mark them
as "internal definitions".

The comment in any case certainly is silly. It dates back to Jan 2007
in our internal version control system. The tilepro hv headers in
question are largely frozen at this point in our internal "upstream"
repository where we take these headers from, so it doesn't seem worth
the effort to try to improve the comments there, nor worth adding skew
between those headers and the community headers.

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com

2014-07-10 18:50:54

by Nicholas Krause

[permalink] [raw]
Subject: Re: [PATCH] tile: Add underscores to defintions

On Thu, Jul 10, 2014 at 2:19 PM, Chris Metcalf <[email protected]> wrote:
> On 7/5/2014 12:32 AM, Nicholas Krause wrote:
>>
>> In file drv_xgbe_impl.h I fixed the definitions for Size Small,
>> Size Large, Size Jumbo to have underscores before and after to
>> follow kernel coding style for internel defentions.
>>
>> Signed-off-by: Nicholas Krause <[email protected]>
>> ---
>> arch/tile/include/hv/drv_xgbe_impl.h | 15 +++------------
>
>
> I don't think there is a kernel coding style for internal definitions.
> It's true that double-underscores mean "more raw" in many places, like
> for things like set_bit() vs __set_bit(), but that's not really
> applicable here. These files are not exported outside of the kernel
> in any case so we don't need any special treatment for them to mark them
> as "internal definitions".
>
> The comment in any case certainly is silly. It dates back to Jan 2007
> in our internal version control system. The tilepro hv headers in
> question are largely frozen at this point in our internal "upstream"
> repository where we take these headers from, so it doesn't seem worth
> the effort to try to improve the comments there, nor worth adding skew
> between those headers and the community headers.
>
> --
> Chris Metcalf, Tilera Corp.
> http://www.tilera.com
Chris ,
Would you like me to remove this comment in a patch instead?
Cheers Nick

2014-07-10 19:15:59

by Chris Metcalf

[permalink] [raw]
Subject: Re: [PATCH] tile: Add underscores to defintions

On 7/10/2014 2:50 PM, Nick Krause wrote:
> On Thu, Jul 10, 2014 at 2:19 PM, Chris Metcalf <[email protected]> wrote:
>> On 7/5/2014 12:32 AM, Nicholas Krause wrote:
>>> In file drv_xgbe_impl.h I fixed the definitions for Size Small,
>>> Size Large, Size Jumbo to have underscores before and after to
>>> follow kernel coding style for internel defentions.
>>>
>>> Signed-off-by: Nicholas Krause <[email protected]>
>>> ---
>>> arch/tile/include/hv/drv_xgbe_impl.h | 15 +++------------
>>
>> I don't think there is a kernel coding style for internal definitions.
>> It's true that double-underscores mean "more raw" in many places, like
>> for things like set_bit() vs __set_bit(), but that's not really
>> applicable here. These files are not exported outside of the kernel
>> in any case so we don't need any special treatment for them to mark them
>> as "internal definitions".
>>
>> The comment in any case certainly is silly. It dates back to Jan 2007
>> in our internal version control system. The tilepro hv headers in
>> question are largely frozen at this point in our internal "upstream"
>> repository where we take these headers from, so it doesn't seem worth
>> the effort to try to improve the comments there, nor worth adding skew
>> between those headers and the community headers.
>>
>> --
>> Chris Metcalf, Tilera Corp.
>> http://www.tilera.com
> Chris ,
> Would you like me to remove this comment in a patch instead?
> Cheers Nick

No, as I said, better not to add skew between our internal copies
of the header and the community version. Thanks anyway.

--
Chris Metcalf, Tilera Corp.
http://www.tilera.com