Messages in this thread | | | Date | Wed, 25 May 2011 22:45:23 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH V2] x86, UV: Reformat uv_mmrs.h - no code changes |
| |
* Jack Steiner <steiner@sgi.com> wrote:
> No code changes. Reformat file to eliminate errors caught > by checkpatch.pl > > Signed-off-by: Jack Steiner <steiner@sgi.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
| |