From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756216AbcIUPxF (ORCPT ); Wed, 21 Sep 2016 11:53:05 -0400 Received: from relay1.sgi.com ([192.48.180.66]:51666 "EHLO relay.sgi.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754426AbcIUPxC (ORCPT ); Wed, 21 Sep 2016 11:53:02 -0400 Date: Wed, 21 Sep 2016 10:53:00 -0500 From: andrew banman To: Ingo Molnar Cc: Andrew Banman , 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 Message-ID: <20160921155300.GF159521@stormcage.americas.sgi.com> References: <1474410720-82890-1-git-send-email-abanman@sgi.com> <1474410720-82890-10-git-send-email-abanman@sgi.com> <20160921125527.GA6424@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160921125527.GA6424@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 21, 2016 at 02:55:27PM +0200, Ingo Molnar wrote: > > * Andrew Banman 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 > > Acked-by: Mike Travis > > Acked-by: Dimitri Sivanich > > --- > > 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