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: 56573 Delivered-To: patch@linaro.org Received: by 10.112.155.196 with SMTP id vy4csp1217508lbb; Mon, 16 Nov 2015 01:47:04 -0800 (PST) X-Received: by 10.195.18.6 with SMTP id gi6mr34322138wjd.83.1447667224376; Mon, 16 Nov 2015 01:47:04 -0800 (PST) Return-Path: Received: from theia.denx.de (theia.denx.de. [85.214.87.163]) by mx.google.com with ESMTP id e191si24536413wma.104.2015.11.16.01.47.04; Mon, 16 Nov 2015 01:47:04 -0800 (PST) Received-SPF: pass (google.com: domain of u-boot-bounces@lists.denx.de designates 85.214.87.163 as permitted sender) client-ip=85.214.87.163; Authentication-Results: mx.google.com; spf=pass (google.com: domain of u-boot-bounces@lists.denx.de designates 85.214.87.163 as permitted sender) smtp.mailfrom=u-boot-bounces@lists.denx.de; dkim=neutral (body hash did not verify) header.i=@linaro_org.20150623.gappssmtp.com 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? -- Linus, you've indicated that you didn't want to make this change but are OK with me taking it over. In my tree, I've just squashed my patch into yours. Would you like me to remove your author and signed-off-by or would you prefer me to post a fixed up version of your patch? Cheers, Ryan. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot 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();