All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Jack Steiner <steiner@sgi.com>
Cc: tglx@linutronix.de, hpa@zytor.com, x86@kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2] x86, UV: Reformat uv_mmrs.h - no code changes
Date: Wed, 25 May 2011 22:45:23 +0200	[thread overview]
Message-ID: <20110525204523.GA28397@elte.hu> (raw)
In-Reply-To: <20110525161634.GA13451@sgi.com>


* 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

  reply	other threads:[~2011-05-25 20:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-12 13:33 [PATCH] x86, UV: Reformat uv_mmrs.h - no code changes Jack Steiner
2011-05-13 12:40 ` Ingo Molnar
2011-05-25 16:16   ` [PATCH V2] " Jack Steiner
2011-05-25 20:45     ` Ingo Molnar [this message]
2011-05-26 17:17       ` [PATCH V3] " Jack Steiner
2011-05-27 12:40         ` Ingo Molnar
2011-05-27 14:52           ` [PATCH V4] x86, UV: Clean up uv_mmrs.h Jack Steiner
2011-05-30 11:04             ` [tip:x86/uv] " tip-bot for Jack Steiner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110525204523.GA28397@elte.hu \
    --to=mingo@elte.hu \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=steiner@sgi.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.