Re: [PATCH V2] x86, UV: Reformat uv_mmrs.h - no code changes

[Ingo Molnar]

* Jack Steiner <steiner@xxxxxxx> wrote:

> No code changes.  Reformat file to eliminate errors caught
> by checkpatch.pl
> 
> Signed-off-by: Jack Steiner <steiner@xxxxxxx>
> 
> ---
> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/