Re: [PATCH v2 4/4] ndctl: accept DIMM name without -d option

[Dan Williams]

On Thu, Feb 22, 2018 at 12:49 PM, Dave Jiang  wrote:
> Making update-firmware in sync with other ndctl syntax and accept DIMM
> id without using a switch. The -d option will still be accepted. Now
> it will accept multiple dimm ids as well as the "all" option.
> i.e.
> ndctl update-firmware -f file.bin nmem0 nmem1
>

This is duplicating the infrastructure that exists in ndctl/dimm.c
let's drop this patch and proceed with merging firmware update into
that file.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH v2 4/4] ndctl: accept DIMM name without -d option

[Dave Jiang]

Making update-firmware in sync with other ndctl syntax and accept DIMM
id without using a switch. The -d option will still be accepted. Now
it will accept multiple dimm ids as well as the "all" option.
i.e.
ndctl update-firmware -f file.bin nmem0 nmem1

Signed-off-by: Dave Jiang 
---
 ndctl/update.c |  119 +---
 1 file changed, 78 insertions(+), 41 deletions(-)

diff --git a/Documentation/ndctl/ndctl-update-firmware.txt 
b/Documentation/ndctl/ndctl-update-firmware.txt
index d742302..6d74c11 100644
--- a/Documentation/ndctl/ndctl-update-firmware.txt
+++ b/Documentation/ndctl/ndctl-update-firmware.txt
@@ -8,7 +8,18 @@ ndctl-update-firmware - provides updating of NVDIMM firmware
 SYNOPSIS
 
 [verse]
-'ndctl update-firmware' -f  -d 
+'ndctl update-firmware'  [..] []
+
+This command updates the persistent DIMM's firmware.
+
+OPTIONS
+---
+-f::
+--firmware::
+   firmware file to be updated.
+-v::
+--verbose::
+   turn on verbose/debug option
 
 COPYRIGHT
 -
diff --git a/ndctl/update.c b/ndctl/update.c
index b148b70..acf2a64 100644
--- a/ndctl/update.c
+++ b/ndctl/update.c
@@ -467,32 +467,6 @@ static int update_firmware(struct update_context *uctx)
return 0;
 }
 
-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;
-   if (!ndctl_dimm_fw_update_supported(dimm)) {
-   error("DIMM firmware update not supported by 
the kernel.");
-   return -ENOTSUP;
-   }
-   uctx->dimm = dimm;
-   rc = update_firmware(uctx);
-   if (rc < 0) {
-   error("Update firmware for dimm %s failed\n",
-   ndctl_dimm_get_devname(dimm));
-   continue;
-   }
-   }
-
-   return rc;
-}
-
 static int verify_fw_file(struct update_context *uctx)
 {
struct stat st;
@@ -538,6 +512,29 @@ cleanup:
return rc;
 }
 
+static int process_dimm(struct update_context *uctx, const char *dimm_id,
+   struct ndctl_dimm *dimm)
+{
+   int rc;
+
+   if (!util_dimm_filter(dimm, dimm_id))
+   return -ENODEV;
+   if (!ndctl_dimm_fw_update_supported(dimm)) {
+   error("DIMM firmware update not supported by the kernel.");
+   return -ENOTSUP;
+   }
+
+   uctx->dimm = dimm;
+   rc = update_firmware(uctx);
+   if (rc < 0) {
+   error("Update firmware for dimm %s failed\n",
+   ndctl_dimm_get_devname(dimm));
+   return rc;
+   }
+
+   return -ENODEV;
+}
+
 int cmd_update_firmware(int argc, const char **argv, void *ctx)
 {
bool verbose;
@@ -554,13 +551,34 @@ int cmd_update_firmware(int argc, const char **argv, void 
*ctx)
"ndctl update_firmware []",
NULL
};
-   int i, rc;
+   int i, rc, err = 0;
+   unsigned long id;
 
argc = parse_options(argc, argv, options, u, 0);
-   for (i = 0; i < argc; i++)
-   error("unknown parameter \"%s\"", argv[i]);
-   if (argc)
+
+   if (argc == 0 && !uctx.dimm_id)
usage_with_options(u, options);
+   if (!uctx.dimm_id) {
+   for (i = 0; i < argc; i++) {
+   if (strcmp(argv[i], "all") == 0) {
+   argv[0] = "all";
+   argc = 1;
+   break;
+   }
+
+   if (sscanf(argv[i], "nmem%lu", ) != 1) {
+   fprintf(stderr,
+   "'%s' is not a valid dimm name\n",
+   argv[i]);
+   err++;
+   }
+   }
+
+   if (err == argc) {
+   usage_with_options(u, options);
+   return -EINVAL;
+   }
+   }
 
if (verbose)
ndctl_set_log_priority(ctx, LOG_DEBUG);
@@ -571,25 +589,44 @@ int cmd_update_firmware(int argc, const char **argv, void 
*ctx)
return -EINVAL;
}
 
-   if (!uctx.dimm_id) {
-   error("No DIMM ID provided");
-   usage_with_options(u, options);
-   return -EINVAL;
-   }
-
rc = verify_fw_file();
if (rc < 0) {
error("Failed to verify firmware file %s", uctx.fw_path);
return rc;
}
 
-   rc