Re: [PATCH v2 2/4] ndctl: add "all" dimm-id option for update-firmware

From: Dan Williams <dan.j.williams@intel.com>
To: Dave Jiang <dave.jiang@intel.com>
Cc: linux-nvdimm@lists.01.org
Subject: Re: [PATCH v2 2/4] ndctl: add "all" dimm-id option for update-firmware
Date: Thu, 22 Feb 2018 22:15:53 -0800	[thread overview]
Message-ID: <CAPcyv4hzoC-ndvmaksz8+B6nZZOBSHFNBkBtQxASqhnPuubq5A@mail.gmail.com> (raw)
In-Reply-To: <151933257703.36856.16473005119268194390.stgit@djiang5-desk3.ch.intel.com>

On Thu, Feb 22, 2018 at 12:49 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> Modifying the code so that when "all" option is passed in, we will process
> all the DIMMs on the platform.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  ndctl/update.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/ndctl/update.c b/ndctl/update.c
> index fc26acf..4fb572d 100644
> --- a/ndctl/update.c
> +++ b/ndctl/update.c
> @@ -471,16 +471,22 @@ static int get_ndctl_dimm(struct update_context *uctx, void *ctx)
>  {
>         struct ndctl_dimm *dimm;
>         struct ndctl_bus *bus;
> +       int rc = -ENODEV;
>
>         ndctl_bus_foreach(ctx, bus)
>                 ndctl_dimm_foreach(bus, dimm) {
>                         if (!util_dimm_filter(dimm, uctx->dimm_id))
>                                 continue;
>                         uctx->dimm = dimm;
> -                       return 0;
> +                       rc = update_firmware(uctx);
> +                       if (rc < 0) {
> +                               error("Update firmware for dimm %s failed\n",
> +                                               ndctl_dimm_get_devname(dimm));
> +                               continue;
> +                       }

This really wants the -b option to available as well, and we would get
that basically for free by moving all of this functionality into
ndctl/dimm.c along with all the other DIMM specific commands. So we
can apply this for now, but update.c really does not deserve to be
standalone command infrastructure away from the existing dimm-specific
utility infrastructure.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm