From patchwork Mon Nov 16 09:46:55 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ryan Harkin X-Patchwork-Id: 544954 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 080851402A2 for ; Mon, 16 Nov 2015 20:47:06 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=linaro_org.20150623.gappssmtp.com header.i=@linaro_org.20150623.gappssmtp.com header.b=jh7VLQwi; dkim-atps=neutral Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id CC53F4B635; Mon, 16 Nov 2015 10:47:03 +0100 (CET) Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LkoUgi4xQD8l; Mon, 16 Nov 2015 10:47:03 +0100 (CET) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 7B7554B65A; Mon, 16 Nov 2015 10:47:03 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id B4E464B65A for ; Mon, 16 Nov 2015 10:47:00 +0100 (CET) Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ef02uklKWV-a for ; Mon, 16 Nov 2015 10:47:00 +0100 (CET) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-yk0-f175.google.com (mail-yk0-f175.google.com [209.85.160.175]) by theia.denx.de (Postfix) with ESMTPS id 31C694B635 for ; Mon, 16 Nov 2015 10:46:57 +0100 (CET) Received: by ykdr82 with SMTP id r82so229167899ykd.3 for ; Mon, 16 Nov 2015 01:46:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro_org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=4Bwj5ieGvSkigB5/yx8WsOT1ZnVwzrPas43VrfWxDNo=; b=jh7VLQwiZZOBJaMftTNsS+5+MQP7VEa3J4RKAXUddrWfU6iP54B2Gmo3u6sgVs7H/N I1J+Twy0jAor/6shhsitTTLeghCgs8I7Z3v0FKCfSz93960loSMxvHYd1ky/aKwOi2Zi rzsAKGZhFBBceTgfGtTKFYuYoUnAjIAziW74A4I2X3hLZB1SPB2aeLK5jXGmM0wsAh1j BqG8tchEJg7PcMy5FISj/w5LSdJ/z3Qrxzp874D6VyuniRyK4bb/YIJU7RANaIURdG4Q 7aLWp3/uNikVqqingGOqHqDuQ6Fk3uQJ0Z2CbW2RJToeo3NRH1hyI8Bw3o0/m9aHOzCw g0Hw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=4Bwj5ieGvSkigB5/yx8WsOT1ZnVwzrPas43VrfWxDNo=; b=mjs5BKIM0S5EDAP/0tSo15r9R23y8HD5oSL8TWownauzOyPbiEg8OW2deHCoEK0a0K JrcNRZ9srFKU9yTPVdoeWkmYC+uiGSkjZgAFiq3QEnvIDfLzJh2R9po3mgc9RY0JdQ6A vGmLizXZ82c33S1hxtMCUjtJJ6+hpNm5wD375otrdPZzmYiDbI2dVFYER1GIja88yvKz 7Rqrme6qjznAdMAdXAtZNBY/HEUXKLaaHaA4QevJSjcgjD4vK206CiQbACgt5bE1JAU+ 2eOH2Db6/hneUZZ/lDztnJbwLo0/tJ89ni2JnINQYCcfCRMOeNa1bmwvl9iwNzljXfB4 Dx1Q== X-Gm-Message-State: ALoCoQlaKl9OLQuyuSFOer3yLXk06Ss4wJ6kELHKY2hAD1yZPyDZJnpml8azjxRxGo+omDxBwP9K MIME-Version: 1.0 X-Received: by 10.129.153.196 with SMTP id q187mr37775200ywg.193.1447667216013; Mon, 16 Nov 2015 01:46:56 -0800 (PST) Received: by 10.37.208.198 with HTTP; Mon, 16 Nov 2015 01:46:55 -0800 (PST) In-Reply-To: References: <1445321140-28976-1-git-send-email-linus.walleij@linaro.org> <20151020153815.GL23893@bill-the-cat> <20151021125030.GK23893@bill-the-cat> Date: Mon, 16 Nov 2015 09:46:55 +0000 Message-ID: From: Ryan Harkin To: Linus Walleij Cc: Tom Rini , u-boot-review@google.com, U-Boot ML Subject: Re: [U-Boot] [PATCH] vexpress64: compile Juno PCIe conditionally X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.15 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" On 13 November 2015 at 13:39, Ryan Harkin wrote: > [trying again with Linus on the to: list] > > Hi Linus, > > Tom asked for a small change to your patch. Would mind having a look > and re-working? > > Thanks, > Ryan. > > On 21 October 2015 at 14:16, Ryan Harkin wrote: >> On 21 October 2015 at 13:50, Tom Rini wrote: >>> On Wed, Oct 21, 2015 at 12:00:03PM +0100, Ryan Harkin wrote: >>>> On 20 October 2015 at 16:38, Tom Rini wrote: >>>> >>>> > On Tue, Oct 20, 2015 at 08:05:40AM +0200, Linus Walleij wrote: >>>> > >>>> > > Only compile in PCIe support if the board really uses it. Provide >>>> > > a stub for the init function if e.g. FVP is being built. >>>> > > >>>> > > Cc: Liviu Dudau >>>> > > Cc: Ryan Harkin >>>> > > Signed-off-by: Linus Walleij >>>> > > --- >>>> > > board/armltd/vexpress64/Makefile | 3 ++- >>>> > > board/armltd/vexpress64/pcie.c | 2 -- >>>> > > board/armltd/vexpress64/pcie.h | 4 ++++ >>>> > > 3 files changed, 6 insertions(+), 3 deletions(-) >>>> > > >>>> > > diff --git a/board/armltd/vexpress64/Makefile >>>> > b/board/armltd/vexpress64/Makefile >>>> > > index a35db401b684..b4391a71249a 100644 >>>> > > --- a/board/armltd/vexpress64/Makefile >>>> > > +++ b/board/armltd/vexpress64/Makefile >>>> > > @@ -5,4 +5,5 @@ >>>> > > # SPDX-License-Identifier: GPL-2.0+ >>>> > > # >>>> > > >>>> > > -obj-y := vexpress64.o pcie.o >>>> > > +obj-y := vexpress64.o >>>> > > +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o >>>> > > diff --git a/board/armltd/vexpress64/pcie.c >>>> > b/board/armltd/vexpress64/pcie.c >>>> > > index 7b999e8ef40b..311c4500e3ff 100644 >>>> > > --- a/board/armltd/vexpress64/pcie.c >>>> > > +++ b/board/armltd/vexpress64/pcie.c >>>> > > @@ -191,7 +191,5 @@ void xr3pci_init(void) >>>> > > >>>> > > void vexpress64_pcie_init(void) >>>> > > { >>>> > > -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO >>>> > > xr3pci_init(); >>>> > > -#endif >>>> > > } >>>> > > diff --git a/board/armltd/vexpress64/pcie.h >>>> > b/board/armltd/vexpress64/pcie.h >>>> > > index 14642f4f5c43..55b276d6af11 100644 >>>> > > --- a/board/armltd/vexpress64/pcie.h >>>> > > +++ b/board/armltd/vexpress64/pcie.h >>>> > > @@ -1,6 +1,10 @@ >>>> > > #ifndef __VEXPRESS64_PCIE_H__ >>>> > > #define __VEXPRESS64_PCIE_H__ >>>> > > >>>> > > +#ifdef CONFIG_TARGET_VEXPRESS64_JUNO >>>> > > void vexpress64_pcie_init(void); >>>> > > +#else >>>> > > +static inline void vexpress64_pcie_init(void) {} >>>> > > +#endif >>>> > > >>>> > > #endif /* __VEXPRESS64_PCIE_H__ */ >>>> > >>>> > Alright, so the versatile platform makes life fun at times. >>>> >>>> >>>> This comes back to the refactoring thread we had recently... >>>> >>>> >>>> >>>> > First, my >>>> > preference is for weak functions (and I _think_ the linker ends up being >>>> > smart enough about them to optimize things away, if not, arrg). Second, >>>> > the question I have is, is this xr3pci_init() bit really a Juno thing or >>>> > a baseboard thing (I assume no) >>>> >>>> >>>> It's sort-of the same thing on Juno. Juno has a baseboard and SoC in one >>>> build, unlike the previous 32-bit Versatile Express motherboard that takes >>>> core tiles with different cores on it. >>>> >>>> Juno R0 has the PCIe controller, but it doesn't work. So the code is safe >>>> to run at this level. Juno R1 has the controller and it works. >>>> >>>> or a going to be the same on the next >>>> > Cortex-Asomething module thing? >>>> >>>> >>>> Juno R2 will have the PCIe controller too and it should hopefully work. >>> >>> But it will also be the A52. Or no, the A72? Or can't say.. >>> >> >> If I knew the answer, "can't say" would probably be the official line. >> But I haven't been told of any plans to change the cores, so I am >> assuming the next Juno respin will be A57/A53 big.LITTLE. Just like >> we have now but with fixes. >> >> >>>> But this controller is not Cortex-Asomething related. Another vendor could >>>> put the same IP block on their board, of course. >>> >>> Right, but it wouldn't be under board/armltd/vexpress64/ either.. >>> >>>> FVP models don't have it and other ARM platforms won't necessarily have it. >>>> >>>> Does that help?! >>> >>> Yes, thanks. I think it's just a style thing then. We don't do a lot >>> of static inline nop functions, we do __weak functions in the main file >>> (and comment about what it should be doing in a real function) and then >>> provide the strong version in another file. So just the pcie.h part >>> needs changing then. >> I'm not familiar with how __weak functions work, but reading Tom's advice and grepping the code leads me to the diff below. Tom, is this what you were looking for? Acked-by: Linus Walleij diff --git a/board/armltd/vexpress64/Makefile b/board/armltd/vexpress64/Makefile index a35db40..b4391a7 100644 --- a/board/armltd/vexpress64/Makefile +++ b/board/armltd/vexpress64/Makefile @@ -5,4 +5,5 @@ # SPDX-License-Identifier: GPL-2.0+ # -obj-y := vexpress64.o pcie.o +obj-y := vexpress64.o +obj-$(CONFIG_TARGET_VEXPRESS64_JUNO) += pcie.o diff --git a/board/armltd/vexpress64/pcie.c b/board/armltd/vexpress64/pcie.c index 7b999e8..311c450 100644 --- a/board/armltd/vexpress64/pcie.c +++ b/board/armltd/vexpress64/pcie.c @@ -191,7 +191,5 @@ void xr3pci_init(void) void vexpress64_pcie_init(void) { -#ifdef CONFIG_TARGET_VEXPRESS64_JUNO xr3pci_init(); -#endif } diff --git a/board/armltd/vexpress64/vexpress64.c b/board/armltd/vexpress64/vexpress64.c index f4e8084..09a3166 100644 --- a/board/armltd/vexpress64/vexpress64.c +++ b/board/armltd/vexpress64/vexpress64.c @@ -28,6 +28,8 @@ U_BOOT_DEVICE(vexpress_serials) = { .platdata = &serial_platdata, }; +__weak void vexpress64_pcie_init(void) {} + int board_init(void) { vexpress64_pcie_init();