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 AD651C433F5 for ; Tue, 7 Dec 2021 18:13:14 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 83B3380F89; Tue, 7 Dec 2021 19:13:11 +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="RZbLKjJt"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 283D68069E; Tue, 7 Dec 2021 19:13:10 +0100 (CET) Received: from mail-ed1-x531.google.com (mail-ed1-x531.google.com [IPv6:2a00:1450:4864:20::531]) (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 3E48E80F89 for ; Tue, 7 Dec 2021 19:13:06 +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=tomm.merciai@gmail.com Received: by mail-ed1-x531.google.com with SMTP id l25so60125902eda.11 for ; Tue, 07 Dec 2021 10:13:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=rKzp/hdFHPYfy0zac3kHBa3XDzTU2GGfGdZkCiAF6so=; b=RZbLKjJtXER/9zeE7py9PhciZnd2OYROqWSVgz8wovh5QtYkS0skbosRtnHhDs+GJc MXyFnyzNFlYo76FVS/7M46OagN/vAdZKMpX33AVhk+ZiBkF5+OwWVOpXYJS+R/m0ESTl tL9Gximps0jAa2YKFwRVuV1kj5ySj8FsYaZVUXh1j6/dLCTw5NITWTtqYcfIrc3HvjNS TZIu0LxnLO+zxY6Le3/HxeS0hOO6JTPPB11ERdOcSyuQ/dbU4R3pkNjomxNDBXd30BJS z0FLeomdKYohI2qe6xl+o5egvm2PoIkRiznIdmgS/RblMt0j5gLwn4+H6KYx4LdlIcKw pVYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=rKzp/hdFHPYfy0zac3kHBa3XDzTU2GGfGdZkCiAF6so=; b=mhN3dDtcTiRZB8yWlYUccqJxdiaxs7CCLn1h0exRg5GuacaO2RZxCcWk8aF+kDnlMy QUIqmK8YhAPzHlfelmSr06vgSsQgIN38uqj5DC912ZDEgDvKdU49+UZRzRHjudz5YBgB jUNB6IJAl4kkxeBs+9YW5pcsTyqnb/I7v9XCxRxXy4AmalBhAsqlxFuAubizGr2azEWZ w7nEZbyvX4iBlhA8rA3iQorZd3CmMgomjzbqXJD2/wjTzvHCTXK7eJtguojdaESEDjC8 oSsnQwHeG7JtvuF5C4KO4h7wvGYs1fCxTBx3OJSxacSU3FqTiRg7JVObPT0NHeQa3bXc PweQ== X-Gm-Message-State: AOAM532Pw7Bwkgp9zVVpEOQX90IW5/qRzHPaIzbxOjDvcM686PrrXX1F fBXX2MN8g4QNWUZeLxLBo88= X-Google-Smtp-Source: ABdhPJxlLnURY7EByCy8rF7Rz+CZw8cA1r1RFTAQARwSzYGbmwO0gdvVpT/d2EOr8asQdAZt38L0sA== X-Received: by 2002:a17:907:250f:: with SMTP id y15mr1166339ejl.0.1638900785744; Tue, 07 Dec 2021 10:13:05 -0800 (PST) Received: from tom-desktop ([37.183.178.57]) by smtp.gmail.com with ESMTPSA id gz10sm183195ejc.38.2021.12.07.10.13.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Dec 2021 10:13:05 -0800 (PST) Date: Tue, 7 Dec 2021 19:13:02 +0100 From: Tommaso Merciai To: Marek =?iso-8859-1?Q?Beh=FAn?= Cc: michael@amarulasolutions.com, andrey.zhizhikin@leica-geosystems.com, Stefano Babic , Fabio Estevam , "NXP i.MX U-Boot Team" , Peng Fan , Ye Li , Marek Vasut , Simon Glass , Frieder Schrempf , Harald Seiler , "Ying-Chun Liu (PaulLiu)" , u-boot@lists.denx.de Subject: Re: [RFC PATCH 3/3] imx: imx8mp_evk: override env_get_location Message-ID: <20211207181302.GA5056@tom-desktop> References: <20211126174331.5928-1-tomm.merciai@gmail.com> <20211126174331.5928-4-tomm.merciai@gmail.com> <20211126190021.7d8428af@thinkpad> <20211128174711.GA4938@tom-desktop> <20211129131744.28234486@thinkpad> <20211130202318.GA2813@tom-desktop> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20211130202318.GA2813@tom-desktop> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.38 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 Tue, Nov 30, 2021 at 09:23:18PM +0100, Tommaso Merciai wrote: > On Mon, Nov 29, 2021 at 01:17:44PM +0100, Marek Behún wrote: > > On Sun, 28 Nov 2021 18:47:11 +0100 > > Tommaso Merciai wrote: > > > > > On Fri, Nov 26, 2021 at 07:00:21PM +0100, Marek Behún wrote: > > > > On Fri, 26 Nov 2021 18:43:31 +0100 > > > > Tommaso Merciai wrote: > > > > > > > > > Override env_get_location function at board level, previously dropped > > > > > down from soc.c > > > > > > > > > > References: > > > > > - commit f1575f23df1ef704051f218d5bc4aeeb20c2c542 > > > > > > > > > > Signed-off-by: Tommaso Merciai > > > > > --- > > > > > Changes since v1: > > > > > - Remove wrong env_get_offset function from commit > > > > > > > > > > board/freescale/imx8mp_evk/imx8mp_evk.c | 41 +++++++++++++++++++++++++ > > > > > 1 file changed, 41 insertions(+) > > > > > > > > > > diff --git a/board/freescale/imx8mp_evk/imx8mp_evk.c b/board/freescale/imx8mp_evk/imx8mp_evk.c > > > > > index 62096c24fb..6b2eeaf152 100644 > > > > > --- a/board/freescale/imx8mp_evk/imx8mp_evk.c > > > > > +++ b/board/freescale/imx8mp_evk/imx8mp_evk.c > > > > > @@ -5,6 +5,7 @@ > > > > > > > > > > #include > > > > > #include > > > > > +#include > > > > > #include > > > > > #include > > > > > #include > > > > > @@ -17,6 +18,7 @@ > > > > > #include > > > > > #include > > > > > #include > > > > > +#include > > > > > > > > > > DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > @@ -32,6 +34,45 @@ static iomux_v3_cfg_t const wdog_pads[] = { > > > > > MX8MP_PAD_GPIO1_IO02__WDOG1_WDOG_B | MUX_PAD_CTRL(WDOG_PAD_CTRL), > > > > > }; > > > > > > > > > > +enum env_location env_get_location(enum env_operation op, int prio) > > > > > +{ > > > > > + enum boot_device dev = get_boot_device(); > > > > > + enum env_location env_loc = ENVL_UNKNOWN; > > > > > + > > > > > + if (prio) > > > > > + return env_loc; > > > > > + > > > > > + switch (dev) { > > > > > +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH > > > > > + case QSPI_BOOT: > > > > > + env_loc = ENVL_SPI_FLASH; > > > > > + break; > > > > > +#endif > > > > > +#ifdef CONFIG_ENV_IS_IN_NAND > > > > > + case NAND_BOOT: > > > > > + env_loc = ENVL_NAND; > > > > > + break; > > > > > +#endif > > > > > +#ifdef CONFIG_ENV_IS_IN_MMC > > > > > + case SD1_BOOT: > > > > > + case SD2_BOOT: > > > > > + case SD3_BOOT: > > > > > + case MMC1_BOOT: > > > > > + case MMC2_BOOT: > > > > > + case MMC3_BOOT: > > > > > + env_loc = ENVL_MMC; > > > > > + break; > > > > > +#endif > > > > > + default: > > > > > +#if defined(CONFIG_ENV_IS_NOWHERE) > > > > > + env_loc = ENVL_NOWHERE; > > > > > +#endif > > > > > > > > Using > > > > if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE)) > > > > instead of #ifdefs is preferred because the compiler then warns about > > > > bugs even in the disabled codepaths (that's why checkpatch complains > > > > about #ifdefs). > > > > > > > > I know that this cannot be combined with switch() in a simple way, > > > > though. > > > > > > > > Do you need to use switch? Can't you rewrite it to use if()s and use > > > > IS_ENABLED()? > > > > > > > > Marek > > > > > > Hi Marek, > > > Thanks for your review. What do you think about this solution? > > > > > > enum env_location env_get_location(enum env_operation op, int prio) > > > { > > > enum boot_device dev = get_boot_device(); > > > enum env_location env_loc = ENVL_UNKNOWN; > > > > > > if (prio) > > > return env_loc; > > > > > > if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH) && dev == QSPI_BOOT) { > > > env_loc = ENVL_SPI_FLASH; > > > } > > > else if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND) && dev == NAND_BOOT) { > > > env_loc = ENVL_NAND; > > > } > > > else if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) { > > > switch (dev) { > > > case SD1_BOOT: > > > case SD2_BOOT: > > > case SD3_BOOT: > > > case MMC1_BOOT: > > > case MMC2_BOOT: > > > case MMC3_BOOT: > > > env_loc = ENVL_MMC; > > > break; > > > default: > > > break; > > > } > > > } > > > else if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE)) { > > > env_loc = ENVL_MMC; > > > } > > > > > > return env_loc; > > > } > > > > Thanks, this looks ok, just run it through checkpatch to correct the > > indentation of 'case' statements and put 'else if' on the same line as > > '}': > > > > if () { > > } else if () { > > } ... > > > > Marek > > Thanks Marek for your review. Sent v2. > > Let me know, > Tommaso Hi Marek, Have you had the time to check v2 yet? Thanks, tommaso