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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 1BF3CC433EF for ; Fri, 28 Jan 2022 05:27:28 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5A8908349B; Fri, 28 Jan 2022 06:27:26 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=sholland.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=sholland.org header.i=@sholland.org header.b="GUiZ7Qus"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.b="lEfBNQD3"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 17536834E4; Fri, 28 Jan 2022 06:27:25 +0100 (CET) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 56F9E8343B for ; Fri, 28 Jan 2022 06:27:21 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=sholland.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=samuel@sholland.org Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id 97A7B5C0138; Fri, 28 Jan 2022 00:27:17 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Fri, 28 Jan 2022 00:27:17 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sholland.org; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm1; bh=HVS14nP3Z513F7 gNf9Lexr4aP+DFP7kjZRsqrVg/8IE=; b=GUiZ7QusxaCH3FrVmz3wWzcQBby8i/ a7/RVSy6qtNqsK9ynFz/9Gws4STACahHnOH+2s3mRJAAD9gJ0alS+xEyeWzonuGR ajFTCmfjDIOY7FiiVfzdcfbQw+IOwAaHGAkVOH6h7p5nxW4on6Vb2HpjHS+65iwv Y0xybQiD8YS6KnuviVSmhhCSqB7nZ9uSfCZ05ySs6zrAlGaP18jNj5ToqiNTSEow 3PZuFreVNFs3TkYhy1dJjrNsZ1XfSMH7UGCRKnCCjKMStWkDMu+KY7J/605tUWTT MXNJA2dopUPrX3c5kT/98GQmroFF4merImWFoziHdhyDpKVenalbvYsQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=HVS14nP3Z513F7gNf9Lexr4aP+DFP7kjZRsqrVg/8 IE=; b=lEfBNQD3sVVK1PncirudgrYZRGD1e1kBDiGUPb2Mv7xDT95CpWSXuPKlN pcf07+nhh4HhXwpTqT5OAIbgS692poj7uzxoljq4IlGgQQXeS6nUnv4SazjNp9rl R6949VQfpAqFDFqqOSEj0GZAob4Hr7GJb1m78RZ2YPYH2+cQrl1oDlxEgARb2s3b wMbjG7rFijjRYHXkuIh7c0w45rUsc5QTtL4d7KnD/HxfibKggRwtVk5MWSdI0ZaZ IsAR5tR/IcPR19Gk5ZugEdFKB9IhT1gISiG8cz7QB7CZ8qGeI5k0LiWolVg5wfSY V67Sjoe8Eczx7ha1TcYTyrRKEaDMw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrfeeggdektdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefuvfhfhffkffgfgggjtgfgsehtjeertddtfeejnecuhfhrohhmpefurghmuhgv lhcujfholhhlrghnugcuoehsrghmuhgvlhesshhhohhllhgrnhgurdhorhhgqeenucggtf frrghtthgvrhhnpeefgfeuhefgtedtiedtueduvddufefhgeefvdekleelkeeflefhveel vdffgfevfeenucffohhmrghinheprghllhifihhnnhgvrhhtvggthhdrtghomhenucevlh hushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehsrghmuhgvlhes shhhohhllhgrnhgurdhorhhg X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 28 Jan 2022 00:27:15 -0500 (EST) Subject: Re: [PATCH 3/5] sunxi: move early "SRAM setup" into separate file To: Andre Przywara Cc: Jagan Teki , Tom Rini , Simon Glass , Jernej Skrabec , Chen-Yu Tsai , Icenowy Zheng , Jesse Taube , u-boot@lists.denx.de References: <20220125011517.7773-1-andre.przywara@arm.com> <20220125011517.7773-4-andre.przywara@arm.com> <90798f8e-a397-c907-ddd4-475951b47da6@sholland.org> <20220128005937.7353301c@slackpad.fritz.box> From: Samuel Holland Message-ID: Date: Thu, 27 Jan 2022 23:27:15 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20220128005937.7353301c@slackpad.fritz.box> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.5 at phobos.denx.de X-Virus-Status: Clean On 1/27/22 6:59 PM, Andre Przywara wrote: > On Mon, 24 Jan 2022 20:56:12 -0600 > Samuel Holland wrote: > > Hi Samuel, > >> On 1/24/22 7:15 PM, Andre Przywara wrote: >>> Currently we do some magic "SRAM setup" MMIO writes in s_init(), copied >>> from the original BSP U-Boot. The comment speaks of this being required >>> before DRAM access gets enabled, but there is no indication that this >>> would actually be required that early. >>> >>> Move this out of s_init(), into board_init_f(). Since this actually only >>> affects a very few older SoCs, the actual code goes into the cpu/armv7 >>> directory, to move it out of the way for all other SoCs. >>> >>> This also uses the opportunity to convert some #ifdefs over to the fancy >>> IS_ENABLED() macros used in actual C code. >>> >>> We keep the s_init() stub around for now, since armv8's lowlevel_init >>> still relies on it. >>> >>> Signed-off-by: Andre Przywara >>> --- >>> arch/arm/cpu/armv7/sunxi/Makefile | 3 +++ >>> arch/arm/cpu/armv7/sunxi/sram.c | 45 +++++++++++++++++++++++++++++++ >>> arch/arm/mach-sunxi/board.c | 38 +++++--------------------- >>> 3 files changed, 54 insertions(+), 32 deletions(-) >>> create mode 100644 arch/arm/cpu/armv7/sunxi/sram.c >>> >>> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile >>> index 1d40d6a18dc..ad11be78632 100644 >>> --- a/arch/arm/cpu/armv7/sunxi/Makefile >>> +++ b/arch/arm/cpu/armv7/sunxi/Makefile >>> @@ -10,6 +10,9 @@ obj-y += timer.o >>> obj-$(CONFIG_MACH_SUN6I) += tzpc.o >>> obj-$(CONFIG_MACH_SUN8I_H3) += tzpc.o >>> >>> +obj-$(CONFIG_MACH_SUN6I) += sram.o >>> +obj-$(CONFIG_MACH_SUN8I) += sram.o >>> + >>> ifndef CONFIG_SPL_BUILD >>> obj-$(CONFIG_ARMV7_PSCI) += psci.o >>> endif >>> diff --git a/arch/arm/cpu/armv7/sunxi/sram.c b/arch/arm/cpu/armv7/sunxi/sram.c >>> new file mode 100644 >>> index 00000000000..19395cce17c >>> --- /dev/null >>> +++ b/arch/arm/cpu/armv7/sunxi/sram.c >>> @@ -0,0 +1,45 @@ >>> +// SPDX-License-Identifier: GPL-2.0+ >>> +/* >>> + * (C) Copyright 2012 Henrik Nordstrom >>> + * >>> + * (C) Copyright 2007-2011 >>> + * Allwinner Technology Co., Ltd. >>> + * Tom Cubie >>> + * >>> + * SRAM init for older sunxi SoCs. >>> + */ >>> + >>> +#include >>> +#include >>> +#include >>> + >>> +void sunxi_sram_init(void) >>> +{ >>> + /* >>> + * Undocumented magic taken from boot0, without this DRAM >>> + * access gets messed up (seems cache related). >>> + * The boot0 sources describe this as: "config ema for cache sram" >>> + * Newer SoCs (A83T, H3 and anything beyond) don't need this anymore. >>> + */ >>> + if (IS_ENABLED(CONFIG_MACH_SUN6I)) >>> + setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800); >>> + >>> + if (IS_ENABLED(CONFIG_MACH_SUN8I)) { >>> + uint version; >>> + >>> + /* Unlock sram version info reg, read it, relock */ >>> + setbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15)); >>> + version = readl(SUNXI_SRAMC_BASE + 0x24) >> 16; >>> + clrbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15)); >> >> This is an open-coded version of sunxi_get_sram_id(). > > Indeed, thanks. Needs a prototype, though, I just picked one header file > in all of this mess ;-) > >>> + >>> + if (IS_ENABLED(CONFIG_MACH_SUN8I_A23)) { >>> + if (version == 0x1650) >>> + setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800); >>> + else /* 0x1661 ? */ >>> + setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0); >>> + } else if (IS_ENABLED(CONFIG_MACH_SUN8I_A33)) { >>> + if (version != 0x1667) >>> + setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0); >>> + } >>> + } >>> +} >>> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c >>> index 8667ddf58e3..42ec02d96e3 100644 >>> --- a/arch/arm/mach-sunxi/board.c >>> +++ b/arch/arm/mach-sunxi/board.c >>> @@ -186,38 +186,6 @@ SPL_LOAD_IMAGE_METHOD("FEL", 0, BOOT_DEVICE_BOARD, spl_board_load_image); >>> >>> void s_init(void) >>> { >>> - /* >>> - * Undocumented magic taken from boot0, without this DRAM >>> - * access gets messed up (seems cache related). >>> - * The boot0 sources describe this as: "config ema for cache sram" >>> - */ >>> -#if defined CONFIG_MACH_SUN6I >>> - setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800); >>> -#elif defined CONFIG_MACH_SUN8I >>> - __maybe_unused uint version; >>> - >>> - /* Unlock sram version info reg, read it, relock */ >>> - setbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15)); >>> - version = readl(SUNXI_SRAMC_BASE + 0x24) >> 16; >>> - clrbits_le32(SUNXI_SRAMC_BASE + 0x24, (1 << 15)); >>> - >>> - /* >>> - * Ideally this would be a switch case, but we do not know exactly >>> - * which versions there are and which version needs which settings, >>> - * so reproduce the per SoC code from the BSP. >>> - */ >>> -#if defined CONFIG_MACH_SUN8I_A23 >>> - if (version == 0x1650) >>> - setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0x1800); >>> - else /* 0x1661 ? */ >>> - setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0); >>> -#elif defined CONFIG_MACH_SUN8I_A33 >>> - if (version != 0x1667) >>> - setbits_le32(SUNXI_SRAMC_BASE + 0x44, 0xc0); >>> -#endif >>> - /* A83T BSP never modifies SUNXI_SRAMC_BASE + 0x44 */ >>> - /* No H3 BSP, boot0 seems to not modify SUNXI_SRAMC_BASE + 0x44 */ >>> -#endif >>> } >>> >>> #define SUNXI_INVALID_BOOT_SOURCE -1 >>> @@ -312,8 +280,14 @@ u32 spl_boot_device(void) >>> return sunxi_get_boot_device(); >>> } >>> >>> +__weak void sunxi_sram_init(void) >>> +{ >>> +} >> >> In configurations other than MACH_SUN6/8I, the strong definition of >> sunxi_sram_init() is also an empty function, so this is no more efficient than >> unconditionally compiling sram.c. > > I know, but I put this here to be able to keep this SRAM cruft in > cpu/armv7/sunxi, and avoid compiling this file for everyone else (ARM9, > ARMv8, RISC-V). Ah, right, that makes sense. I missed the directory change here. > Do you have a better idea? I actually tried several ways before, and > this seemed to be the cleanest. Yes, we do a call to a ret, for most > boards, but we probably have bigger problems. No, I am fine with your method. Regards, Samuel > And I consider this just the beginning of a sunxi cleanup journey, so > we can revisit this later. > > Cheers, > Andre > >> >>> + >>> void board_init_f(ulong dummy) >>> { >>> + sunxi_sram_init(); >>> + >>> #if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I_H3 >>> /* Enable non-secure access to some peripherals */ >>> tzpc_init(); >>> >> >