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 5C75AC77B75 for ; Wed, 17 May 2023 08:56:57 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 045D8861E8; Wed, 17 May 2023 10:56:55 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com 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 36B7684A00; Wed, 17 May 2023 10:56:54 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id 21C84866C1 for ; Wed, 17 May 2023 10:56:51 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 086CA1FB; Wed, 17 May 2023 01:57:35 -0700 (PDT) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3DC823F663; Wed, 17 May 2023 01:56:49 -0700 (PDT) Date: Wed, 17 May 2023 09:56:46 +0100 From: Andre Przywara To: Sam Edwards Cc: Samuel Holland , Jagan Teki , u-boot@lists.denx.de, Icenowy Zheng , Jernej Skrabec , Maksim Kiselev Subject: Re: [RFC PATCH 08/17] sunxi: introduce NCAT2 generation model Message-ID: <20230517095646.3f8a7f03@donnerap.cambridge.arm.com> In-Reply-To: <20230517014312.01986aa6@slackpad.lan> References: <20221206004549.29015-1-andre.przywara@arm.com> <20221206004549.29015-9-andre.przywara@arm.com> <20230516220850.7ab973b1@slackpad.lan> <2d8c3fce-dfbf-37ec-f7f1-21684c5d1da6@gmail.com> <20230517014312.01986aa6@slackpad.lan> Organization: ARM X-Mailer: Claws Mail 3.18.0 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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.8 at phobos.denx.de X-Virus-Status: Clean On Wed, 17 May 2023 01:43:12 +0100 Andre Przywara wrote: +Maksim, as he was interested in the U-Boot series as well and had some plans for SPI-NOR booting, IIUC. Cheers, Andre > On Tue, 16 May 2023 17:53:38 -0600 > Sam Edwards wrote: > > Hi Sam, > > > On 5/16/23 15:08, Andre Przywara wrote: > > > This whole memory map is somewhat of a legacy. Apart from a few > > > addresses for the SPL needs we shouldn't have those defines at all. > > > Some symbols are needed because there are other macros using them, > > > although these then are eventually unused. > > > I have some patches to remove most of the symbols, and patch 14/17 > > > demonstrates some idea how to pin this down to what's really needed. > > > > > > For this particular case: this was copied from the H6 memory map, some > > > addresses are just plain wrong for the D1 family. I will try to remove > > > them as much as possible, leaving only the ones needed in. > > > > I see - the only "tangible" concern I had was the access to > > prcm->res_cal_ctrl done in > > arch/arm/mach-sunxi/clock_sun50i_h6.c:clock_init_safe > > > > This doesn't appear to upset the silicon but also doesn't seem necessary > > either -- and with how tight of a memory footprint SPL has to fit into, > > What's the particular concern here? Compared to the A64 we are pretty > cool: it's Thumb2 code and we are at around 27KB, at least with my > toolchain. And I haven't tried, but I am pretty sure the BROM > loads more than 32K, as it does on the H6 and H616 already. The U-Boot > build system and the code already supports this - we rely on this for > the H616 - so we can lift the limit anytime, if really needed. > > > I wanted to check whether this was just something undocumented or dead > > code that needed to be removed. It sounds like it's mostly the latter. > > I haven't checked if the vendor boot0 does this. I am pretty sure there > is a PRCM block, it's just regularly not mentioned in the manuals. > > > > So where did you see problems? If you would (wrongly) reference > > > PortL somewhere in SPL GPIO code, it would use a wrong pointer, but at > > > least the code would still compile fine, wouldn't it? > > > > The specific patch I had to apply (to arch/arm/mach-sunxi/board.c) was: > > /* Update PIO power bias configuration by copy hardware > > detected value */ > > val = readl(SUNXI_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_VAL); > > writel(val, SUNXI_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_SEL); > > - val = readl(SUNXI_R_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_VAL); > > - writel(val, SUNXI_R_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_SEL); > > + if (SUNXI_R_PIO_BASE) { > > + val = readl(SUNXI_R_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_VAL); > > + writel(val, SUNXI_R_PIO_BASE + SUN50I_H6_GPIO_POW_MOD_SEL); > > + } > > Ah, I see, I indeed missed that. We seem to define all symbols anyway, > so we can even lose the #ifdef and use proper if's here. > Will incorporate that in the next drop. > > > With SUNXI_R_PIO_BASE being 0, this was actually attempting to write to > > BROM. This might also be something that doesn't really upset the > > silicon, though: my debug environment is a concolic emulator I quickly > > hacked up to trace MMIO accesses, and it flagged the write to BROM as an > > error. It was easier to patch the SPL than to have the emulator ignore > > the error (and verify that the T113 was cool with it). > > Ah yeah, the Allwinner interconnect is pretty relaxed about those > things: accesses to addresses with no device behind them are usually > ignored (RAZ/WI), where other platform might throw an external abort. > Writes to ROM areas are ignored as well. > > > Since this kind of extraneous/erroneous init code tends to remain > > undetected when the symbols they need are dummied-out like this, I > > figured I'd give a nudge in the direction of instead *removing* the > > symbols where appropriate and fixing whatever breaks -- especially since > > we really need to be thrifty about SPL size. But that might also be > > something that happens in a later cleanup pass when the patchset is > > being prepared for upstream inclusion. :) > > > > > P.S. Could you try the github post? Then compiled and booted fine for > > > me, and includes the DRAM code as well now: > > > https://github.com/apritzel/u-boot/commits/t113s-mq-r-WIP > > > > Ooh, more up-to-date code, thanks for the link! I'll switch to using > > this instead going forward. My pulls from that branch might be > > relatively infrequent > > Don't worry, I won't push to this anymore. > > > since I'm also working on some patches for better > > Clang compatibility concurrent with the efforts here. Is this email > > thread a good venue for feedback against that branch or would you prefer > > that I use GitHub issues instead? > > Please use this thread here, if you find something still wrong in the > branch. I just pushed it to github since someone asked for a fixed > and complete version, and I didn't have time to prepare a proper post > again. > > I will hopefully post a proper version for upstreaming in the next days. > > > > Warm regards, > > Sam > > > > P.S. My target is the BMC on the Turing Pi 2 board. > > Ah, interesting, didn't know that this is now a BMC - for a > SoC from Allwinner's arch nemesis Rockchip ;-) > > > They have the same > > SoC and (apparently) UART console configuration, but the differences end > > there: in particular, my target supports boot from either/both > > microSD+SPI-NAND. I might have to start pushing for room for SPI drivers > > in the SPL soon. :) > > We already have SPI(-NOR) booting support, check > arch/arm/mach-sunxi/spl_spi_sunxi.c. This code is very small, and just > needs to be updated to cover the D1/T113 SPI controller, which is > slightly different. See > https://lore.kernel.org/linux-arm-kernel/20230507150345.1971083-1-bigunclemax@gmail.com/ > for the Linux SPI bits. > Regarding SPI-*NAND*: there is > https://patchwork.ozlabs.org/user/todo/uboot/?series=322733 > which is supposed to allow loading U-Boot proper from SPI-NAND. I > haven't tested it yet, and wasn't overly happy with the refactoring, but > would appreciate any kind of review or test. > > Cheers, > Andre