From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5561AC433F5 for ; Fri, 29 Oct 2021 12:00:43 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B907660E0B for ; Fri, 29 Oct 2021 12:00:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org B907660E0B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 75B57835AF; Fri, 29 Oct 2021 14:00:39 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 8D59E835AF; Fri, 29 Oct 2021 14:00:36 +0200 (CEST) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 6C3038321B for ; Fri, 29 Oct 2021 14:00:32 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=maz@kernel.org Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id C55E4610EA; Fri, 29 Oct 2021 12:00:30 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mgQYe-002PQ0-Ii; Fri, 29 Oct 2021 13:00:28 +0100 Date: Fri, 29 Oct 2021 13:00:28 +0100 Message-ID: <87ilxg9h2b.wl-maz@kernel.org> From: Marc Zyngier To: Michael Walle Cc: u-boot@lists.denx.de, Vladimir Oltean , Hou Zhiqiang , Bharat Gooty , Rayagonda Kokatanur , Simon Glass , Priyanka Jain , Tom Rini Subject: Re: [PATCH 2/2] Revert "arch: arm: use dt and UCLASS_SYSCON to get gic lpi details" In-Reply-To: <9ecafe6c05395d5e382a91c292362e0b@walle.cc> References: <20211027165454.1501398-1-michael@walle.cc> <20211027165454.1501398-3-michael@walle.cc> <87k0hwua9m.wl-maz@kernel.org> <9ecafe6c05395d5e382a91c292362e0b@walle.cc> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: michael@walle.cc, u-boot@lists.denx.de, vladimir.oltean@nxp.com, Zhiqiang.Hou@nxp.com, bharat.gooty@broadcom.com, rayagonda.kokatanur@broadcom.com, sjg@chromium.org, priyanka.jain@nxp.com, trini@konsulko.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 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" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Michael, On Fri, 29 Oct 2021 12:49:21 +0100, Michael Walle wrote: > > Hi, > > thanks for the review, as Tom already mentioned this is just > a revert. I'm still unsure wether I should care or not. Actually, > NXP or Broadcom should. OTOH it would be nice if the kontron_sl28 > can do kexec also with booti (and not just bootefi). Anyway, I > still have some remarks. > > Am 2021-10-28 23:09, schrieb Marc Zyngier: > > On Wed, 27 Oct 2021 17:54:54 +0100, > > Michael Walle wrote: > >> > >> Stop using the device tree as a source for ad-hoc information. > >> > >> This reverts commit 2ae7adc659f7fca9ea65df4318e5bca2b8274310. > >> > >> Signed-off-by: Michael Walle > >> --- > > >> int ls_gic_rd_tables_init(void *blob) > >> { > >> + u64 gic_lpi_base; > >> int ret; > >> > >> - ret = gic_lpi_tables_init(); > >> + gic_lpi_base = ALIGN(gd->arch.resv_ram - GIC_LPI_SIZE, SZ_64K); > >> + ret = fdt_add_resv_mem_gic_rd_tables(blob, gic_lpi_base, > >> GIC_LPI_SIZE); > >> + if (ret) > >> + return ret; > >> + > >> + ret = gic_lpi_tables_init(gic_lpi_base, cpu_numcores()); > > > > This really should fetch the number of CPUs from the DT rather then > > some SoC specific black magic... > > Remember that this is the bootloader. There might be a chicken egg > problem. I guess, usually the bootloader will fixup the number of cores > in the DT. Therefore, if we rely on the DT here, it has to be fixed up > before this code is run. I appreciate that. However... > > Conceptually, I'm unsure if this should actually be a driver > (UCLASS_IRQ). It's just setting up the interrupt controller, it doesn't > provide any ops. So it might well be called from somewhere else instead > of binding as a driver to the gic. > > If it will be a driver, then the call to gic_lpi_tables_init() should > go away. Instead the driver should just set the tables up. > > If its not a driver, then gic_lpi_tables_init() (maybe renamed to > gic_v3_lpi_tables_init()) should work without anything else and > it should scan the device tree for the compatible node and fetch > all needed information to set up the tables. Exactly. This thing doesn't provide *any* service to u-boot itself. It just grabs some memory, point a couple of registers at it, and flip a bit. There is no actual ITS driver, so nothing makes use of it. So this can be run as late as you want, long after you have worked out how many CPUs you really have. Which means this can be done in a SoC-agnostic way. > In any case, all this code should be guarded by a Kconfig option which > clearly states *why* this is needed. Even more: if it is selected, it should be possible to disable this at runtime (environment variable?) and leave the OS in charge of it. This really should an opt-in, instead of being forced on the users. Thanks, M. -- Without deviation from the norm, progress is not possible.