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 7A065C6FD1D for ; Mon, 27 Mar 2023 19:31:30 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5C95985727; Mon, 27 Mar 2023 21:31:28 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; secure) header.d=xs4all.nl header.i=@xs4all.nl header.b="sF948h7z"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id DB5CF860ED; Mon, 27 Mar 2023 21:31:26 +0200 (CEST) Received: from ewsoutbound.kpnmail.nl (ewsoutbound.kpnmail.nl [195.121.94.183]) (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 A699D80A52 for ; Mon, 27 Mar 2023 21:31:23 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=mark.kettenis@xs4all.nl X-KPN-MessageId: e5c62943-ccd5-11ed-b08d-005056992ed3 Received: from smtp.kpnmail.nl (unknown [10.31.155.7]) by ewsoutbound.so.kpn.org (Halon) with ESMTPS id e5c62943-ccd5-11ed-b08d-005056992ed3; Mon, 27 Mar 2023 21:30:57 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=xs4all01; h=subject:to:from:message-id:date; bh=ZfA3PMw9yj2BebvF/7EF1l8/tfCOuB5DPVuJ7PLNK+4=; b=sF948h7zqGQ4xC97bkNqw/hfknS6nkNW2eoOGK+mYR98BJrHo+6T/F+KfbpFXKMP4Fh5ZC5zoLyMC JUymXQW5AG3iadEGS5VCkNLS/aM+yUw83G3nM8CG8f0CKbJuBXK0pnBJO3sLWSlRnGJfSp/rzXCvhB r9KbYed4LZGzvHkxaCJiMmwWb10hWKugr+qnKgn7FblqyzoOehl/kBvn4fq2aSiH0A1yQi6i0HFJ6L iuVWUP7v2cSy4gfO2fS9d9oq0Slwk4LXM+w65cmKU2VxQTngfFFTY/bZXw1C3zIZwFMxS/bfYslw0L Fblrd0cEmpjPOy6/LMWJGoWnFPmttiA== X-KPN-MID: 33|T4xn05SYMqTDGwDYgPVgJJXvCYCPt22FaHyucspGwkM7/Lf5p0cGfkby+A+yyy1 +VT2aDkRqf7BtS4v9dy/9LsOaFGhOW3rTx/dKFkIhFnk= X-KPN-VerifiedSender: Yes X-CMASSUN: 33|XKiaMDNQtIUbyj2aPE/6MJc1ytHLvmDUDU7W0AA6ALYMF31lIRWMNhTMOHUmcOZ XIKo3m0MzEe+7liHd3fEZmA== X-Originating-IP: 80.61.163.207 Received: from bloch.sibelius.xs4all.nl (80-61-163-207.fixed.kpn.net [80.61.163.207]) by smtp.xs4all.nl (Halon) with ESMTPSA id f4a2f9f2-ccd5-11ed-a8ef-005056998788; Mon, 27 Mar 2023 21:31:23 +0200 (CEST) Date: Mon, 27 Mar 2023 21:31:22 +0200 Message-Id: <874jq62dpx.fsf@bloch.sibelius.xs4all.nl> From: Mark Kettenis To: Konrad Dybcio Cc: u-boot@lists.denx.de, bhupesh.sharma@linaro.org, konrad.dybcio@linaro.org, dsankouski@gmail.com, kettenis@openbsd.org, sjg@chromium.org, trini@konsulko.com In-Reply-To: <20230324004040.572525-7-konrad.dybcio@linaro.org> (message from Konrad Dybcio on Fri, 24 Mar 2023 01:40:40 +0100) Subject: Re: [PATCH 7/7] arm: Migrate Apple M1 to save_prev_bl_data References: <20230324004040.572525-1-konrad.dybcio@linaro.org> <20230324004040.572525-7-konrad.dybcio@linaro.org> 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 > From: Konrad Dybcio > Date: Fri, 24 Mar 2023 01:40:40 +0100 Hi Konrad, > Mark's and Dzmitry's approaches come down to the same thing.. Let's > unify them by first removing the static keyword from the common file > to allow the variable to be reused, then renaming "reg0" to the more > sensible fw_dtb_pointer coming from the Apple file and finally remove > the mach-apple implementation of this very thing and enable the common > approach in the respective defconfig. > > Only build-tested. > > Signed-off-by: Konrad Dybcio > --- > > arch/arm/lib/save_prev_bl_data.c | 14 +++++++------- > arch/arm/mach-apple/Makefile | 1 - > arch/arm/mach-apple/lowlevel_init.S | 17 ----------------- > configs/apple_m1_defconfig | 1 + > 4 files changed, 8 insertions(+), 25 deletions(-) > delete mode 100644 arch/arm/mach-apple/lowlevel_init.S > > diff --git a/arch/arm/lib/save_prev_bl_data.c b/arch/arm/lib/save_prev_bl_data.c > index f7b23faf0d66..a127fec1f149 100644 > --- a/arch/arm/lib/save_prev_bl_data.c > +++ b/arch/arm/lib/save_prev_bl_data.c > @@ -15,7 +15,7 @@ > #include > #include > > -static ulong reg0 __section(".data"); > +ulong fw_dtb_pointer __section(".data"); > > /** > * Save x0 register value, assuming previous bootloader set it to > @@ -23,7 +23,7 @@ static ulong reg0 __section(".data"); > */ > void save_boot_params(ulong r0) > { > - reg0 = r0; > + fw_dtb_pointer = r0; > save_boot_params_ret(); I suppose this works, but it is a bit strange sice save_boot_params_ret is just a label that we're supposed to jump back to, not a function. > } > > @@ -51,24 +51,24 @@ int save_prev_bl_data(void) > int node; > u64 initrd_start_prop; > > - if (!is_addr_accessible((phys_addr_t)reg0)) > + if (!is_addr_accessible((phys_addr_t)fw_dtb_pointer)) > return -ENODATA; > > - fdt_blob = (struct fdt_header *)reg0; > + fdt_blob = (struct fdt_header *)fw_dtb_pointer; > if (!fdt_valid(&fdt_blob)) { > - pr_warn("%s: address 0x%lx is not a valid fdt\n", __func__, reg0); > + pr_warn("%s: address 0x%lx is not a valid fdt\n", __func__, fw_dtb_pointer); > return -ENODATA; > } > > if (IS_ENABLED(CONFIG_SAVE_PREV_BL_FDT_ADDR)) > - env_set_addr("prevbl_fdt_addr", (void *)reg0); > + env_set_addr("prevbl_fdt_addr", (void *)fw_dtb_pointer); > if (!IS_ENABLED(CONFIG_SAVE_PREV_BL_INITRAMFS_START_ADDR)) > return 0; > > node = fdt_path_offset(fdt_blob, "/chosen"); > if (!node) { > pr_warn("%s: chosen node not found in device tree at addr: 0x%lx\n", > - __func__, reg0); > + __func__, fw_dtb_pointer); > return -ENODATA; > } > /* And we have no use for these additional environment variables and I'd prefer not to add them. > diff --git a/arch/arm/mach-apple/Makefile b/arch/arm/mach-apple/Makefile > index 50b465b9473f..a775d8866ad4 100644 > --- a/arch/arm/mach-apple/Makefile > +++ b/arch/arm/mach-apple/Makefile > @@ -1,6 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0+ > > obj-y += board.o > -obj-y += lowlevel_init.o > obj-y += rtkit.o > obj-$(CONFIG_NVME_APPLE) += sart.o > diff --git a/arch/arm/mach-apple/lowlevel_init.S b/arch/arm/mach-apple/lowlevel_init.S > deleted file mode 100644 > index e1c0d91cef2c..000000000000 > --- a/arch/arm/mach-apple/lowlevel_init.S > +++ /dev/null > @@ -1,17 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0+ */ > -/* > - * (C) Copyright 2021 Mark Kettenis > - */ > - > -.align 8 > -.global fw_dtb_pointer > -fw_dtb_pointer: > - .quad 0 > - > -.global save_boot_params > -save_boot_params: > - /* Stash DTB pointer passed by m1n1 */ > - adr x1, fw_dtb_pointer > - str x0, [x1] > - > - b save_boot_params_ret > diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig > index b4ecf73cbc78..eb0addb741c5 100644 > --- a/configs/apple_m1_defconfig > +++ b/configs/apple_m1_defconfig > @@ -3,6 +3,7 @@ CONFIG_ARCH_APPLE=y > CONFIG_DEFAULT_DEVICE_TREE="t8103-j274" > CONFIG_SYS_LOAD_ADDR=0x0 > CONFIG_USE_PREBOOT=y > +CONFIG_SAVE_PREV_BL_FDT_ADDR=y > # CONFIG_DISPLAY_CPUINFO is not set > CONFIG_DISPLAY_BOARDINFO_LATE=y > CONFIG_BOARD_LATE_INIT=y > -- > 2.40.0 > >