From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Thu, 11 Mar 2021 21:45:19 -0700
Subject: [PATCH v2 3/5] sysinfo: Require that sysinfo_detect be called
 before other methods
In-Reply-To: <20210305201225.3753841-4-sean.anderson@seco.com>
References: <20210305201225.3753841-1-sean.anderson@seco.com>
 <20210305201225.3753841-4-sean.anderson@seco.com>
Message-ID: <CAPnjgZ2Qipp6HSQthx1HfBkRNL3XXaQOk4q=ytey4F7fve4o6g@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Fri, 5 Mar 2021 at 13:12, Sean Anderson <sean.anderson@seco.com> wrote:
>
> This has the uclass enforce calling detect() before other methods.  This
> allows drivers to cache information in detect() and perform (cheaper)
> retrieval in the other accessors. This also modifies the only instance
> where this sequencing was not followed.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
> Changes in v2:
> - Enforce sysinfo detect ordering in uclass. Users must still call
>   sysinfo_detect beforehand.
> - Modify sysinfo test to check for detect() ordering.
>
>  common/spl/spl_fit.c             |  4 ++++
>  drivers/sysinfo/sysinfo-uclass.c | 25 ++++++++++++++++++++++++-
>  include/sysinfo.h                | 26 +++++++++++++++++---------
>  test/dm/sysinfo.c                | 23 ++++++++++++++---------
>  4 files changed, 59 insertions(+), 19 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

But please see below.

>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index a6ad094e91..4d17582af5 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -116,6 +116,10 @@ static int spl_fit_get_image_name(const void *fit, int images,
>                  * no string in the property for this index. Check if the
>                  * sysinfo-level code can supply one.
>                  */
> +               rc = sysinfo_detect(sysinfo);
> +               if (rc)
> +                       return rc;
> +
>                 rc = sysinfo_get_fit_loadable(sysinfo, index - i - 1, type,
>                                               &str);
>                 if (rc && rc != -ENOENT)
> diff --git a/drivers/sysinfo/sysinfo-uclass.c b/drivers/sysinfo/sysinfo-uclass.c
> index 6df58fe160..f034ee0870 100644
> --- a/drivers/sysinfo/sysinfo-uclass.c
> +++ b/drivers/sysinfo/sysinfo-uclass.c
> @@ -15,19 +15,29 @@ int sysinfo_get(struct udevice **devp)
>
>  int sysinfo_detect(struct udevice *dev)
>  {
> +       int ret;
> +       bool *detected = dev_get_uclass_priv(dev);

I would argue for a struct with a bool in it here, instead of a raw bool.

Regards,
Simon