From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757103AbcIUMzh (ORCPT ); Wed, 21 Sep 2016 08:55:37 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:36242 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754266AbcIUMzf (ORCPT ); Wed, 21 Sep 2016 08:55:35 -0400 Date: Wed, 21 Sep 2016 14:55:27 +0200 From: Ingo Molnar To: Andrew Banman Cc: 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: <20160921125527.GA6424@gmail.com> References: <1474410720-82890-1-git-send-email-abanman@sgi.com> <1474410720-82890-10-git-send-email-abanman@sgi.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1474410720-82890-10-git-send-email-abanman@sgi.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * 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