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 A604DC433F5 for ; Tue, 9 Nov 2021 08:09:26 +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 2CB876112D for ; Tue, 9 Nov 2021 08:09:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 2CB876112D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com 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 83B40838F9; Tue, 9 Nov 2021 09:09:24 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com 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; unprotected) header.d=gmail.com header.i=@gmail.com header.b="HQNHjSsE"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 3DA41838FA; Tue, 9 Nov 2021 09:09:23 +0100 (CET) Received: from mail-oi1-x22e.google.com (mail-oi1-x22e.google.com [IPv6:2607:f8b0:4864:20::22e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 9B923838B0 for ; Tue, 9 Nov 2021 09:09:19 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=rfried.dev@gmail.com Received: by mail-oi1-x22e.google.com with SMTP id o4so32366418oia.10 for ; Tue, 09 Nov 2021 00:09:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=1xH4Y/aNLDVnpXdpeY0VCDlKlkboPrbKH6AfD9QUOLg=; b=HQNHjSsE6VeFFm6Dje4NB+pMRZtadS6nTj91fsbBFhgUuIWWIe6ajHkf2eaEYnB69K gUFR8wKl7Czpzg8ol3k3krPZIPiftye+Ru6Qi2aUdo0aON5CZT7GUTgloMzh8YGTU9Wt 4/ZuY9hjQn+qZ9X6WIQVa5M6cAe6ZkgxkQADMEfJn29B7QceVicEeC7APE8r04Yg3HP1 S9P3TmPPnJOAvSPjjfY2vKa9jV3EtGwH812FwzS92m2+KL9lmOc25vyTVuTAGJOLS60u 4QU8JAkwNmtyrxx1r0cO08iB347eGGBv0FWSDcWiAc1JKLk0/r0ki5AOInnxfOr5nKB3 W2tA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=1xH4Y/aNLDVnpXdpeY0VCDlKlkboPrbKH6AfD9QUOLg=; b=c+aG/1CiU6r5T/GGj78RLB+OSP0xmeIrR9+8/nsZbbwYj+OI1NqrCB5n5vLx01+8LB wR1BUFqofNQLGK4Xmofi7CtHUQ9otdgU9BQkn6zCQrQcjMObV/aUD8nrGeSX9ZngV3+T UycqTsIHg+rb9AHfuQb03m31JoLdZMuChBjfToIUuracG/XOoUBESPn+6cZemBs6kmmC /ieeDcRQf5wzjUUdeqUNxhNIXktVIu53hWrgA1nTjrPIiJZpR11qbI8SB1KMXO8kbet3 zqzeLGqp0JMEosEDoI/LlEulaliLSai39FfxMcbmAdLvZBmKTBdsPZShsk1lDt/nIPfa fODg== X-Gm-Message-State: AOAM532pYBgDgDBSSdZzuYYtb6uJaHhukH+SQ/FcuE1NdoVZpWQVLxK0 F7a0xgHOqt8F610ANy655PVqIqkyb3GRviEOiEE= X-Google-Smtp-Source: ABdhPJz6oyvcHdiK9o55laXnASjwWgmzKQjtu9mNjpuUSv5UjtNbS5+9zhsF6CZsKZltax0XR4Ha7OuV8PoAJJ1sm6E= X-Received: by 2002:a05:6808:bd6:: with SMTP id o22mr3912212oik.151.1636445358360; Tue, 09 Nov 2021 00:09:18 -0800 (PST) MIME-Version: 1.0 References: <20211014184811.482560-1-sjg@chromium.org> <20211014124803.v3.16.Ibd54f1a4872bc99ab0822d2c44492dc6c7581b60@changeid> In-Reply-To: <20211014124803.v3.16.Ibd54f1a4872bc99ab0822d2c44492dc6c7581b60@changeid> From: Ramon Fried Date: Tue, 9 Nov 2021 10:09:07 +0200 Message-ID: Subject: Re: [PATCH v3 16/18] pxe: Refactor sysboot to have one helper To: Simon Glass Cc: U-Boot Mailing List , Patrice Chotard , Artem Lapkin , Tom Rini , Joe Hershberger , Heinrich Schuchardt , Peter Hoyes Content-Type: text/plain; charset="UTF-8" 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 On Thu, Oct 14, 2021 at 9:51 PM Simon Glass wrote: > > The only difference between the three helpers is the filesystem type. > Factor this out and call the filesystem functions directly, instead of > through the command-line interpreter. This allows the file size to be > obtained directly, instead of via an environment variable. > > We cannot do the same thing with PXE's tftpboot since there is no API > at present to obtain information about the file that was read. So there > is no point in changing pxe_getfile_func to use a ulong for the address, > for example. > > This is as far as the refactoring can go for the present. > > Signed-off-by: Simon Glass > --- > > (no changes since v1) > > cmd/sysboot.c | 94 ++++++++++++++++++++------------------------------- > 1 file changed, 36 insertions(+), 58 deletions(-) > > diff --git a/cmd/sysboot.c b/cmd/sysboot.c > index 6344ecd357b..04c07020269 100644 > --- a/cmd/sysboot.c > +++ b/cmd/sysboot.c > @@ -6,63 +6,40 @@ > #include > #include > > -static char *fs_argv[5]; > - > -static int do_get_ext2(struct pxe_context *ctx, const char *file_path, > - char *file_addr, ulong *sizep) > +/** > + * struct sysboot_info - useful information for sysboot helpers > + * > + * @fstype: Filesystem type (FS_TYPE_...) > + * @ifname: Interface name (e.g. "ide", "scsi") > + * @dev_part_str is in the format: > + * .: where is the device number, > + * is the optional hardware partition number and > + * is the partition number > + */ > +struct sysboot_info { > + int fstype; > + const char *ifname; > + const char *dev_part_str; > +}; > + > +static int sysboot_read_file(struct pxe_context *ctx, const char *file_path, > + char *file_addr, ulong *sizep) > { > -#ifdef CONFIG_CMD_EXT2 > + struct sysboot_info *info = ctx->userdata; > + loff_t len_read; > + ulong addr; > int ret; > > - fs_argv[0] = "ext2load"; > - fs_argv[3] = file_addr; > - fs_argv[4] = (void *)file_path; > - > - if (!do_ext2load(ctx->cmdtp, 0, 5, fs_argv)) > - return 1; > - ret = pxe_get_file_size(sizep); > + addr = simple_strtoul(file_addr, NULL, 16); > + ret = fs_set_blk_dev(info->ifname, info->dev_part_str, info->fstype); > if (ret) > - return log_msg_ret("tftp", ret); > -#endif > - return -ENOENT; > -} > - > -static int do_get_fat(struct pxe_context *ctx, const char *file_path, > - char *file_addr, ulong *sizep) > -{ > -#ifdef CONFIG_CMD_FAT > - int ret; > - > - fs_argv[0] = "fatload"; > - fs_argv[3] = file_addr; > - fs_argv[4] = (void *)file_path; > - > - if (!do_fat_fsload(ctx->cmdtp, 0, 5, fs_argv)) > - return 1; > - ret = pxe_get_file_size(sizep); > + return ret; > + ret = fs_read(file_path, addr, 0, 0, &len_read); > if (ret) > - return log_msg_ret("tftp", ret); > -#endif > - return -ENOENT; > -} > - > -static int do_get_any(struct pxe_context *ctx, const char *file_path, > - char *file_addr, ulong *sizep) > -{ > -#ifdef CONFIG_CMD_FS_GENERIC > - int ret; > - > - fs_argv[0] = "load"; > - fs_argv[3] = file_addr; > - fs_argv[4] = (void *)file_path; > + return ret; > + *sizep = len_read; > > - if (!do_load(ctx->cmdtp, 0, 5, fs_argv, FS_TYPE_ANY)) > - return 1; > - ret = pxe_get_file_size(sizep); > - if (ret) > - return log_msg_ret("tftp", ret); > -#endif > - return -ENOENT; > + return 0; > } > > /* > @@ -74,9 +51,9 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, int argc, > char *const argv[]) > { > unsigned long pxefile_addr_r; > - pxe_getfile_func getfile; > struct pxe_context ctx; > char *pxefile_addr_str; > + struct sysboot_info info; > char *filename; > int prompt = 0; > int ret; > @@ -106,24 +83,25 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, int argc, > } > > if (strstr(argv[3], "ext2")) { > - getfile = do_get_ext2; > + info.fstype = FS_TYPE_EXT; > } else if (strstr(argv[3], "fat")) { > - getfile = do_get_fat; > + info.fstype = FS_TYPE_FAT; > } else if (strstr(argv[3], "any")) { > - getfile = do_get_any; > + info.fstype = FS_TYPE_ANY; > } else { > printf("Invalid filesystem: %s\n", argv[3]); > return 1; > } > - fs_argv[1] = argv[1]; > - fs_argv[2] = argv[2]; > + info.ifname = argv[1]; > + info.dev_part_str = argv[2]; > > if (strict_strtoul(pxefile_addr_str, 16, &pxefile_addr_r) < 0) { > printf("Invalid pxefile address: %s\n", pxefile_addr_str); > return 1; > } > > - if (pxe_setup_ctx(&ctx, cmdtp, getfile, NULL, true, filename)) { > + if (pxe_setup_ctx(&ctx, cmdtp, sysboot_read_file, &info, true, > + filename)) { > printf("Out of memory\n"); > return CMD_RET_FAILURE; > } > -- > 2.33.0.1079.g6e70778dc9-goog > Reviewed-by: Ramon Fried