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 5D7E3C43334 for ; Mon, 20 Jun 2022 17:33:49 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7FC2E820E2; Mon, 20 Jun 2022 19:33:37 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="C5apRL0O"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id ADC5F81F8B; Mon, 20 Jun 2022 19:33:33 +0200 (CEST) Received: from mail-qv1-xf2a.google.com (mail-qv1-xf2a.google.com [IPv6:2607:f8b0:4864:20::f2a]) (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 B15DC8207A for ; Mon, 20 Jun 2022 19:33:28 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qv1-xf2a.google.com with SMTP id y14so11554308qvs.10 for ; Mon, 20 Jun 2022 10:33:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Tqqw/ZQxwlNhDot0mnuaanwojK5GqG57akBguh3XEJo=; b=C5apRL0OpR7BM7T4WLODDzuKx5MuFZqEZz1q/JIX+sYL6aznSOXvPXHN4PojHd1S8W i2MhiUSdqNPLYN+lEQ0BzDOtJpHwwbpqq0iLcPjNv6zN8DGS5gFENw6PFKpuIJcGRlQQ qp4n03V9gErF4GYelN1F3hqpRmcHj7azxGQHM= 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:in-reply-to; bh=Tqqw/ZQxwlNhDot0mnuaanwojK5GqG57akBguh3XEJo=; b=2FYwA3h2yb+5VFYU9FKalZd8IDwg/I7SxgCjai4eaT1hIjFSXSHCpLvi1jRNcJV/UB mFqELP+9G7v4UDOO938bQLQXIPAEFvFVoh0Neisf9Lh1E7stGzr+XFVUZVAApJMWrsc1 BaQ+VpCWcW8+9LyxDeB2fG4DHYyO3JeQhdsyvXb6uAfw8XAPiHmrB1c1IxQPGD0dEoDC s7hNLm/cvMxyxqmMnP4lNJnPbF22js1olQm1x9a7aIqYh8nOOTKr4idNkosVAA3+WVZN C0OEuHEkLjCdjNjIlPBbDMgksNjsTv4IWYksKqnkL6JPHbOWp3aGbOBVHtiCnJJOBlVD dyjw== X-Gm-Message-State: AJIora/xea8uQMzzuu2OZs7pwvoouvnipSN3/XM3itfqW1SXg0sJTh1a nF0yIQbhRqRoSZ3q1Gee3mbJlw== X-Google-Smtp-Source: AGRyM1uRKfTM1k6xeloSw/9GQLt9TkjzJu/BzkhJvCzdG9UAPMrea78QCnMmtiUAyO5q260jWLILpA== X-Received: by 2002:a05:622a:305:b0:305:f25:9350 with SMTP id q5-20020a05622a030500b003050f259350mr20443412qtw.507.1655746407119; Mon, 20 Jun 2022 10:33:27 -0700 (PDT) Received: from bill-the-cat (cpe-65-184-195-139.ec.res.rr.com. [65.184.195.139]) by smtp.gmail.com with ESMTPSA id w10-20020ac84d0a000000b00304dec6452csm10730823qtv.78.2022.06.20.10.33.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Jun 2022 10:33:26 -0700 (PDT) Date: Mon, 20 Jun 2022 13:33:24 -0400 From: Tom Rini To: Holger Brunck Cc: Aleksandar Gerasimovski , Francis Laniel , "u-boot@lists.denx.de" , Marek Behun , Michael Nazzareno Trimarchi , Simon Glass , Wolfgang Denk , Harald Seiler , Heiko Schocher Subject: Re: [RFC PATCH v4 28/28] board: keymile: common: Use environment to store IVM_* variables. Message-ID: <20220620173324.GP2484912@bill-the-cat> References: <20220616223158.9225-1-francis.laniel@amarulasolutions.com> <20220616223158.9225-29-francis.laniel@amarulasolutions.com> <20220617144821.GY2484912@bill-the-cat> <20220620153503.GN2484912@bill-the-cat> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="MaJCi81mteg5dvZU" Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett 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.6 at phobos.denx.de X-Virus-Status: Clean --MaJCi81mteg5dvZU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jun 20, 2022 at 04:08:32PM +0000, Holger Brunck wrote: > > > > On Fri, Jun 17, 2022 at 12:31:58AM +0200, Francis Laniel wrote: > > > > > > > > > These boards used set_local_var() to store some variables as loca= l shell. > > > > > They then used get_local_var() to retrieve the variables values. > > > > > > > > > > Instead of using local shell variables, they should use > > > > > environment ones (like a majority of board). > > > > > So, this patch converts using local variables to environment ones. > > > > > > > > > > > why do we need to change that? It is intended that we use this hush > > > variable infrastructure from u-boot (common/hush.c) for our IVM data > > > and not the standard env. We read the IVM at boot time and store these > > > values in RAM. It is not intended to store them permanently in the > > > flash or wherever the environment is saved. Especially in our case we > > > have some boards where the environment is in a i2c EEPROM and we don't > > > want to write down to the EEPROM each time when the board is starting. > >=20 > > So, the whole series is about updating hush to bring in a new baseline = version of > > the shell, from busybox. > >=20 >=20 > Ok. >=20 > > > > > Signed-off-by: Francis Laniel > > > > > > > > > > --- > > > > > board/keymile/common/common.c | 8 ++++---- > > > > > board/keymile/common/ivm.c | 9 +-------- > > > > > 2 files changed, 5 insertions(+), 12 deletions(-) > > > > > > > > > > diff --git a/board/keymile/common/common.c > > > > > b/board/keymile/common/common.c index 3999f48719..72939af36e > > > > > 100644 > > > > > --- a/board/keymile/common/common.c > > > > > +++ b/board/keymile/common/common.c > > > > > @@ -219,7 +219,7 @@ static int do_setboardid(struct cmd_tbl > > > > > *cmdtp, int > > > > flag, int argc, > > > > > unsigned char buf[32]; > > > > > char *p; > > > > > > > > > > - p =3D get_local_var("IVM_BoardId"); > > > > > + p =3D env_get("IVM_BoardId"); > > > > > if (!p) { > > > > > printf("can't get the IVM_Boardid\n"); > > > > > return 1; > > > > > @@ -228,7 +228,7 @@ static int do_setboardid(struct cmd_tbl > > > > > *cmdtp, int > > > > flag, int argc, > > > > > env_set("boardid", (char *)buf); > > > > > printf("set boardid=3D%s\n", buf); > > > > > > > > > > - p =3D get_local_var("IVM_HWKey"); > > > > > + p =3D env_get("IVM_HWKey"); > > > > > if (!p) { > > > > > printf("can't get the IVM_HWKey\n"); > > > > > return 1; > > > > > @@ -272,14 +272,14 @@ static int do_checkboardidhwk(struct cmd_tbl > > > > *cmdtp, int flag, int argc, > > > > > * first read out the real inventory values, these values are > > > > > * already stored in the local hush variables > > > > > */ > > > > > - p =3D get_local_var("IVM_BoardId"); > > > > > + p =3D env_get("IVM_BoardId"); > > > > > if (!p) { > > > > > printf("can't get the IVM_Boardid\n"); > > > > > return 1; > > > > > } > > > > > rc =3D strict_strtoul(p, 16, &ivmbid); > > > > > > > > > > - p =3D get_local_var("IVM_HWKey"); > > > > > + p =3D env_get("IVM_HWKey"); > > > > > if (!p) { > > > > > printf("can't get the IVM_HWKey\n"); > > > > > return 1; > > > > > diff --git a/board/keymile/common/ivm.c > > > > > b/board/keymile/common/ivm.c index 67db0c50f4..e266d7ce81 100644 > > > > > --- a/board/keymile/common/ivm.c > > > > > +++ b/board/keymile/common/ivm.c > > > > > @@ -44,14 +44,7 @@ static int ivm_calc_crc(unsigned char *buf, int > > > > > len) > > > > > > > > > > static int ivm_set_value(char *name, char *value) { > > > > > - char tempbuf[256]; > > > > > - > > > > > - if (value) { > > > > > - sprintf(tempbuf, "%s=3D%s", name, value); > > > > > - return set_local_var(tempbuf, 0); > > > > > - } > > > > > - unset_local_var(name); > > > > > - return 0; > > > > > + return env_set(name, value); > > > > > > this means we are now writing always down to the permanent environment > > > or? And this I would really like to avoid in our case. > >=20 > > Note that "env_set" does not force a save of the running environment. >=20 > Ah yes you are right. But still for the first boot of our board we will c= all saveenv > to store an initial environment and with this change it would also write = down > the IVM data which is currently only stored temporary in RAM. >=20 > > These variables will be exposed to the CLI run-time, which I am not sur= e if they > > are today, so if the user then does "saveenv" they will be written. Th= at I think > > would be a functional difference. > >=20 >=20 > yes exactly and I wonder if this functionality will be also possible afte= r the rework. > I mean our use case (even it seems we are the only ones using it) is quit= e useful > I think. We read out inventory data from an EEPROM and they are parsed and > temporary stored in RAM and then we are able to use them as any other > environment variables without the need to store them permanently. I also = would > like to avoid this as the data should be permanently in the IVM only and = not stored > a second time permanently in flash. So, I'll start by noting that the environment variable flag support isn't as well documented as I would like. But my super quick glance makes me wonder if ENV_FLAGS_VARACCESS_WRITEABLE does what you're thinking about, and if not, if it would be useful to extend the flag support to include a "hidden" flag, or otherwise way to indicate that variables A/B/C should never be saved. This could be useful for other cases as well. --=20 Tom --MaJCi81mteg5dvZU Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmKwr1oACgkQFHw5/5Y0 tyxI3gv/VMBgatSuEDztidbhl8pLwHbq93dhBG1AdG/cJIaQCQpSK6bs3ECf+lIc OJV178VAJI2kEbXhU9dyYRBhQRzbmx11n764aHfpfGhlSF8unQJSYPocbvkLC0Mw MsGYJlhHxwR2WMx66ECWX+xrgKKq83a9VWEjyp5UyExMomi+QK96s8LG0/+NuLhm p0YGroKoji30lnEjURx9tqR7Nk37n5IFv74OWorcvWlb1ljcyXgepFdjNf/ygYiB /wDNTT/vMozVr2de9No4OXX3rP55Gk8WbETxDVtUjgnXKWE6SsD1+FNVxTCnEEhk 8NjfRZa6yxACiQRHpiQWF/MXe5hJJUbv/42ZjH9HeljZj0mGitTV0LqiV0YRv+ll 8DbcqAgezxZApW0jsRiC6V+7wWp7xmUZZyBjKgdMfn00y38ARUf4/dbBrqrsL91K nVw2mLlPOFIcyeo4jleCZOIYlMnQHSFunjd77HYAMTotSFGrsHlJFrxD1YebmZEs c/kB+x/s =k6RP -----END PGP SIGNATURE----- --MaJCi81mteg5dvZU--