From: andrew banman <abanman@sgi.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Banman <abanman@sgi.com>,
mingo@redhat.com, akpm@linux-foundation.org, tglx@linutronix.de,
hpa@zytor.com, travis@sgi.com, rja@sgi.com, sivanich@sgi.com,
x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 9/9] arch/x86/platform/uv: BAU UV4 add version-specific functions
Date: Wed, 21 Sep 2016 10:53:00 -0500 [thread overview]
Message-ID: <20160921155300.GF159521@stormcage.americas.sgi.com> (raw)
In-Reply-To: <20160921125527.GA6424@gmail.com>
On Wed, Sep 21, 2016 at 02:55:27PM +0200, Ingo Molnar wrote:
>
> * Andrew Banman <abanman@sgi.com> wrote:
>
> > Add the UV4-specific function definitions and define an operations struct
> > to implement them in the BAU driver.
> >
> > Many BAU MMRs, although functionally the same, have new addresses on UV4
> > due to hardware changes. Each MMR requires new read/write functions, but
> > their implementation in the driver does not change. Thus, it is enough to
> > enumerate them in the operations struct for the changes to take effect.
> >
> > Signed-off-by: Andrew Banman <abanman@sgi.com>
> > Acked-by: Mike Travis <travis@sgi.com>
> > Acked-by: Dimitri Sivanich <sivanich@sgi.com>
> > ---
> > arch/x86/include/asm/uv/uv_bau.h | 30 ++++++++++++++++++++++++++++++
> > arch/x86/platform/uv/tlb_uv.c | 15 ++++++++++++++-
> > 2 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/uv/uv_bau.h b/arch/x86/include/asm/uv/uv_bau.h
> > index a7a93a5..57ab86d 100644
> > --- a/arch/x86/include/asm/uv/uv_bau.h
> > +++ b/arch/x86/include/asm/uv/uv_bau.h
> > @@ -664,6 +664,16 @@ static inline void write_gmmr_activation(int pnode, unsigned long mmr_image)
> > write_gmmr(pnode, UVH_LB_BAU_SB_ACTIVATION_CONTROL, mmr_image);
> > }
> >
> > +static inline void write_mmr_proc_payload_first(int pnode, unsigned long mmr_image)
> > +{
> > + write_gmmr(pnode, UV4H_LB_PROC_INTD_QUEUE_FIRST, mmr_image);
> > +}
> > +
> > +static inline void write_mmr_proc_payload_last(int pnode, unsigned long mmr_image)
> > +{
> > + write_gmmr(pnode, UV4H_LB_PROC_INTD_QUEUE_LAST, mmr_image);
> > +}
> > +
> > static inline void write_mmr_payload_first(int pnode, unsigned long mmr_image)
> > {
> > write_gmmr(pnode, UVH_LB_BAU_INTD_PAYLOAD_QUEUE_FIRST, mmr_image);
> > @@ -709,6 +719,26 @@ static inline unsigned long read_gmmr_sw_ack(int pnode)
> > return read_gmmr(pnode, UVH_LB_BAU_INTD_SOFTWARE_ACKNOWLEDGE);
> > }
> >
> > +static inline void write_mmr_proc_sw_ack(unsigned long mr)
> > +{
> > + uv_write_local_mmr(UV4H_LB_PROC_INTD_SOFT_ACK_CLEAR, mr);
> > +}
> > +
> > +static inline void write_gmmr_proc_sw_ack(int pnode, unsigned long mr)
> > +{
> > + write_gmmr(pnode, UV4H_LB_PROC_INTD_SOFT_ACK_CLEAR, mr);
> > +}
> > +
> > +static inline unsigned long read_mmr_proc_sw_ack(void)
> > +{
> > + return read_lmmr(UV4H_LB_PROC_INTD_SOFT_ACK_PENDING);
> > +}
> > +
> > +static inline unsigned long read_gmmr_proc_sw_ack(int pnode)
> > +{
> > + return read_gmmr(pnode, UV4H_LB_PROC_INTD_SOFT_ACK_PENDING);
> > +}
> > +
> > static inline void write_mmr_data_config(int pnode, unsigned long mr)
> > {
> > uv_write_global_mmr64(pnode, UVH_BAU_DATA_CONFIG, mr);
> > diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
> > index 56d12eb..470d73c 100644
> > --- a/arch/x86/platform/uv/tlb_uv.c
> > +++ b/arch/x86/platform/uv/tlb_uv.c
> > @@ -36,6 +36,17 @@ static struct bau_operations uv123_bau_ops = {
> > .write_payload_last = write_mmr_payload_last,
> > };
> >
> > +static struct bau_operations uv4_bau_ops = {
> > + .bau_gpa_to_offset = uv_gpa_to_soc_phys_ram,
> > + .read_l_sw_ack = read_mmr_proc_sw_ack,
> > + .read_g_sw_ack = read_gmmr_proc_sw_ack,
> > + .write_l_sw_ack = write_mmr_proc_sw_ack,
> > + .write_g_sw_ack = write_gmmr_proc_sw_ack,
> > + .write_payload_first = write_mmr_proc_payload_first,
> > + .write_payload_last = write_mmr_proc_payload_last,
> > +};
>
> The series looks mostly good to me, only a minor nit: could you please organize
> such initializations vertically?
>
> Something like:
>
> .bau_gpa_to_offset = uv_gpa_to_soc_phys_ram,
> .read_l_sw_ack = read_mmr_proc_sw_ack,
> .read_g_sw_ack = read_gmmr_proc_sw_ack,
> .write_l_sw_ack = write_mmr_proc_sw_ack,
> .write_g_sw_ack = write_gmmr_proc_sw_ack,
> .write_payload_first = write_mmr_proc_payload_first,
> .write_payload_last = write_mmr_proc_payload_last,
>
> would make it more readable. Same might apply to other patches too in the series.
>
> Plus it might make sense to do the same to the existing tunables[] structure as
> well.
>
> Thanks,
>
> Ingo
I will add another cleanup patch to fix the alignment of those code blocks and
resubmit. Thank you for the review, Ingo!
Andrew
next prev parent reply other threads:[~2016-09-21 15:53 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-20 22:31 [PATCH 0/9] arch/x86/platform/uv: add UV4 support to BAU Andrew Banman
2016-09-20 22:31 ` [PATCH 1/9] arch/x86/platform/uv: BAU cleanup: update printks Andrew Banman
2016-09-20 22:31 ` [PATCH 2/9] arch/x86/platform/uv: BAU cleanup: pq_init Andrew Banman
2016-09-20 22:31 ` [PATCH 3/9] arch/x86/platform/uv: BAU replace uv_physnodeaddr Andrew Banman
2016-09-20 22:31 ` [PATCH 4/9] arch/x86/platform/uv: BAU add generic function pointers Andrew Banman
2016-09-20 22:31 ` [PATCH 5/9] arch/x86/platform/uv: BAU use " Andrew Banman
2016-09-20 22:31 ` [PATCH 6/9] arch/x86/platform/uv: BAU UV4 populate uvhub_version Andrew Banman
2016-09-20 22:31 ` [PATCH 7/9] arch/x86/platform/uv: BAU UV4 disable software timeout Andrew Banman
2016-09-20 22:31 ` [PATCH 8/9] arch/x86/platform/uv: BAU UV4 fix payload queue setup Andrew Banman
2016-09-20 22:32 ` [PATCH 9/9] arch/x86/platform/uv: BAU UV4 add version-specific functions Andrew Banman
2016-09-21 12:55 ` Ingo Molnar
2016-09-21 15:53 ` andrew banman [this message]
2016-09-21 14:55 ` [PATCH 0/9] arch/x86/platform/uv: add UV4 support to BAU Thomas Gleixner
2016-09-21 16:09 ` [PATCHv2 0/10] " Andrew Banman
2016-09-21 16:09 ` [PATCH 01/10] arch/x86/platform/uv: BAU cleanup: vertical alignment Andrew Banman
2016-09-22 10:28 ` [tip:x86/platform] x86/platform/uv/BAU: Clean up " tip-bot for Andrew Banman
2016-09-21 16:09 ` [PATCH 02/10] arch/x86/platform/uv: BAU cleanup: update printks Andrew Banman
2016-09-22 10:29 ` [tip:x86/platform] x86/platform/uv/BAU: Clean up and " tip-bot for Andrew Banman
2016-09-21 16:09 ` [PATCH 03/10] arch/x86/platform/uv: BAU cleanup: pq_init Andrew Banman
2016-09-22 10:29 ` [tip:x86/platform] x86/platform/uv/BAU: Clean up pq_init() tip-bot for Andrew Banman
2016-09-21 16:09 ` [PATCH 04/10] arch/x86/platform/uv: BAU replace uv_physnodeaddr Andrew Banman
2016-09-22 10:30 ` [tip:x86/platform] x86/platform/uv/BAU: Convert uv_physnodeaddr() use to uv_gpa_to_offset() tip-bot for Andrew Banman
2016-09-21 16:09 ` [PATCH 05/10] arch/x86/platform/uv: BAU add generic function pointers Andrew Banman
2016-09-22 10:30 ` [tip:x86/platform] x86/platform/uv/BAU: Add " tip-bot for Andrew Banman
2016-09-21 16:09 ` [PATCH 06/10] arch/x86/platform/uv: BAU use " Andrew Banman
2016-09-22 10:31 ` [tip:x86/platform] x86/platform/uv/BAU: Use " tip-bot for Andrew Banman
2016-09-21 16:09 ` [PATCH 07/10] arch/x86/platform/uv: BAU UV4 populate uvhub_version Andrew Banman
2016-09-22 10:31 ` [tip:x86/platform] x86/platform/uv/BAU: Populate ->uvhub_version with UV4 version information tip-bot for Andrew Banman
2016-09-21 16:09 ` [PATCH 08/10] arch/x86/platform/uv: BAU UV4 disable software timeout Andrew Banman
2016-09-22 10:32 ` [tip:x86/platform] x86/platform/uv/BAU: Disable software timeout on UV4 hardware tip-bot for Andrew Banman
2016-09-21 16:09 ` [PATCH 09/10] arch/x86/platform/uv: BAU UV4 fix payload queue setup Andrew Banman
2016-09-22 10:32 ` [tip:x86/platform] x86/platform/uv/BAU: Fix payload queue setup on UV4 hardware tip-bot for Andrew Banman
2016-09-21 16:09 ` [PATCH 10/10] arch/x86/platform/uv: BAU UV4 add version-specific functions Andrew Banman
2016-09-22 10:33 ` [tip:x86/platform] x86/platform/uv/BAU: Add UV4-specific functions tip-bot for Andrew Banman
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=20160921155300.GF159521@stormcage.americas.sgi.com \
--to=abanman@sgi.com \
--cc=akpm@linux-foundation.org \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=rja@sgi.com \
--cc=sivanich@sgi.com \
--cc=tglx@linutronix.de \
--cc=travis@sgi.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).