linux-kernel - Re: [PATCH V2] x86, UV: Reformat uv_mmrs.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110525204523.GA28397@elte.hu>
Date:	Wed, 25 May 2011 22:45:23 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Jack Steiner <steiner@....com>
Cc:	tglx@...utronix.de, hpa@...or.com, x86@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] x86, UV: Reformat uv_mmrs.h - no code changes


* Jack Steiner <steiner@....com> wrote:

> No code changes.  Reformat file to eliminate errors caught
> by checkpatch.pl
> 
> Signed-off-by: Jack Steiner <steiner@....com>
> 
> ---
> V2 - this patch applies on top of "[PATCH] x86, UV: Support for SGI UV2 hub chip".
> 
> I fixed alignment of comments in the structure definitions. All checkpatch.pl
> ERRORS & WARNINGS are also fixed.
> 
> Some of the symbol names are still quite long. The file is based on post-processing
> of verilog definitions that are used for the node controller chip design. Although
> some symbol names are not what I would chose, I would like to maintain compatibility
> with the names used by the chip designers. We have a number of cross-reference
> utilities & having common names is important. Hope this is ok...

I looked at the resulting file and while it improved with this patch, 
it still has obvious problems with things like:

#define UVH_BAU_DATA_CONFIG_VECTOR_SHFT 0
#define UVH_BAU_DATA_CONFIG_VECTOR_MASK 0x00000000000000ffUL
#define UVH_BAU_DATA_CONFIG_DM_SHFT 8
#define UVH_BAU_DATA_CONFIG_DM_MASK 0x0000000000000700UL
#define UVH_BAU_DATA_CONFIG_DESTMODE_SHFT 11
#define UVH_BAU_DATA_CONFIG_DESTMODE_MASK 0x0000000000000800UL
#define UVH_BAU_DATA_CONFIG_STATUS_SHFT 12
#define UVH_BAU_DATA_CONFIG_STATUS_MASK 0x0000000000001000UL
#define UVH_BAU_DATA_CONFIG_P_SHFT 13
#define UVH_BAU_DATA_CONFIG_P_MASK 0x0000000000002000UL
#define UVH_BAU_DATA_CONFIG_T_SHFT 15
#define UVH_BAU_DATA_CONFIG_T_MASK 0x0000000000008000UL
#define UVH_BAU_DATA_CONFIG_M_SHFT 16
#define UVH_BAU_DATA_CONFIG_M_MASK 0x0000000000010000UL
#define UVH_BAU_DATA_CONFIG_APIC_ID_SHFT 32
#define UVH_BAU_DATA_CONFIG_APIC_ID_MASK 0xffffffff00000000UL

and the same mistake repeated all over again. Does this sequence 
really look visually good to you?

Check the enum declarations in include/linux/perf_event.h for 
example. Do you see the visual difference?

Btw., keeping the Verilog cross-reference compatibility is fine IMO.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/