Re: [PATCH v5 3/7] xen/arm: Detect silicon revision and set cap bits accordingly

From: Julien Grall <julien.grall@arm.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: sstabellini@kernel.org, xen-devel@lists.xen.org
Subject: Re: [PATCH v5 3/7] xen/arm: Detect silicon revision and set cap bits accordingly
Date: Wed, 27 Jul 2016 17:09:15 +0100	[thread overview]
Message-ID: <b989a188-5beb-50f4-4090-f0f27e96bd8c@arm.com> (raw)
In-Reply-To: <20160722142433.GD21054@char.us.oracle.com>

Hi Konrad,

On 22/07/16 15:24, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 20, 2016 at 04:25:56PM +0100, Julien Grall wrote:
>> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
>> new file mode 100644
>> index 0000000..fe93beb
>> --- /dev/null
>> +++ b/xen/include/asm-arm/cpuerrata.h
>> @@ -0,0 +1,14 @@
>> +#ifndef __ARM_CPUERRATA_H
>
> Sorry about having engaged the pedantic review mode, but this caught my
> eye.
>
> I thought the style was to prefix it with __ and also postfix it with __:
>
> $ grep "__" *.h
> decode.h:#ifndef __ARCH_ARM_DECODE_H_
> decode.h:#define __ARCH_ARM_DECODE_H_
> decode.h:#endif /* __ARCH_ARM_DECODE_H_ */
> kernel.h:#ifndef __ARCH_ARM_KERNEL_H__
> kernel.h:#define __ARCH_ARM_KERNEL_H__
> kernel.h:#endif /* #ifdef __ARCH_ARM_KERNEL_H__ */
> vtimer.h:#ifndef __ARCH_ARM_VTIMER_H__
> vtimer.h:#define __ARCH_ARM_VTIMER_H__
> vuart.h:#ifndef __ARCH_ARM_VUART_H__
> vuart.h:#define __ARCH_ARM_VUART_H__
> vuart.h:#endif /* __ARCH_ARM_VUART_H__ */
>
> And the include/asm also has have this in in surplus.
>
> It really is minor and I am sorry for even picking up on this, but
> could you add the __ at the end?

I am not sure if there is a strict coding style here. A lot of ARM 
headers (and x86 too) does not have the trailing "__" or only one "_" as 
the first example you gave.

Anyway, I don't mind to add them here.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel