From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751778AbcFDLpU (ORCPT ); Sat, 4 Jun 2016 07:45:20 -0400 Received: from foss.arm.com ([217.140.101.70]:57536 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751032AbcFDLpR (ORCPT ); Sat, 4 Jun 2016 07:45:17 -0400 Date: Sat, 4 Jun 2016 12:45:07 +0100 From: Marc Zyngier To: Shanker Donthineni Cc: , , Thomas Gleixner , Jason Cooper , Vikram Sethi , Philip Elcan Subject: Re: [PATCH V3 1/2] irqchip/gicv3-its: split its_alloc_tables() into two functions Message-ID: <20160604124507.436e5f7e@arm.com> In-Reply-To: <1462827506-23570-2-git-send-email-shankerd@codeaurora.org> References: <1462827506-23570-1-git-send-email-shankerd@codeaurora.org> <1462827506-23570-2-git-send-email-shankerd@codeaurora.org> Organization: ARM Ltd X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; arm-unknown-linux-gnueabihf) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 9 May 2016 15:58:25 -0500 Shanker Donthineni wrote: > The function is getting out of control, it has too many goto > statements and would be too complicated for adding a feature > two-level device table. So, it is time for us to cleanup and > move some of the logic to a separate function without affecting > the existing functionality. > > Signed-off-by: Shanker Donthineni > --- > drivers/irqchip/irq-gic-v3-its.c | 256 ++++++++++++++++++++----------------- > include/linux/irqchip/arm-gic-v3.h | 3 + > 2 files changed, 144 insertions(+), 115 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 6bd881b..b23e00c 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -55,13 +55,15 @@ struct its_collection { > }; > > /* > - * The ITS_BASER structure - contains memory information and cached > - * value of BASER register configuration. > + * The ITS_BASER structure - contains memory information, cached value > + * of BASER register configuration, ioremaped address and page size. > */ > struct its_baser { > + void __iomem *hwreg; > void *base; > u64 val; > u32 order; > + u32 psz; > }; > > /* > @@ -823,27 +825,135 @@ static void its_free_tables(struct its_node *its) > } > } > > +static int its_baser_setup(struct its_node *its, struct its_baser *baser, > + u32 order, u64 indirect) > +{ > + u64 val = readq_relaxed(baser->hwreg); > + u64 type = GITS_BASER_TYPE(val); > + u64 entry_size = GITS_BASER_ENTRY_SIZE(val); > + int psz, alloc_pages; > + u64 cache, shr, tmp; > + void *base; > + > + /* Do first attempt with the requested attributes */ > + cache = baser->val & GITS_BASER_CACHEABILITY_MASK; > + shr = baser->val & GITS_BASER_SHAREABILITY_MASK; > + psz = baser->psz; > + > +retry_alloc_baser: > + alloc_pages = (PAGE_ORDER_TO_SIZE(order) / psz); > + if (alloc_pages > GITS_BASER_PAGES_MAX) { > + pr_warn("ITS@%lx: %s too large, reduce ITS pages %u->%u\n", > + its->phys_base, its_base_type_string[type], > + alloc_pages, GITS_BASER_PAGES_MAX); By the way: as you're changing the output of various messages, please use %pa instead of %lx (and make sure you're passing the parameter by reference...). Thanks, M. -- Jazz is not dead. It just smells funny.