On Thu, Feb 22, 2018 at 12:49 PM, Dave Jiang <[email protected]> wrote: > Adding generic and intel support function to allow check if update firmware > is supported by the kernel. >
Looks good, just one user message I'll fix up on applying... > Signed-off-by: Dave Jiang <[email protected]> > --- > ndctl/lib/firmware.c | 11 +++++++++++ > ndctl/lib/intel.c | 24 ++++++++++++++++++++++++ > ndctl/lib/libndctl.sym | 1 + > ndctl/lib/private.h | 1 + > ndctl/libndctl.h | 1 + > ndctl/update.c | 7 ++++++- > 6 files changed, 44 insertions(+), 1 deletion(-) > > diff --git a/ndctl/lib/firmware.c b/ndctl/lib/firmware.c > index f6deec5..78d753c 100644 > --- a/ndctl/lib/firmware.c > +++ b/ndctl/lib/firmware.c > @@ -107,3 +107,14 @@ ndctl_cmd_fw_xlat_firmware_status(struct ndctl_cmd *cmd) > else > return FW_EUNKNOWN; > } > + > +NDCTL_EXPORT bool > +ndctl_dimm_fw_update_supported(struct ndctl_dimm *dimm) > +{ > + struct ndctl_dimm_ops *ops = dimm->ops; > + > + if (ops && ops->fw_update_supported) > + return ops->fw_update_supported(dimm); > + else > + return false; > +} > diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c > index cee5204..7d976c5 100644 > --- a/ndctl/lib/intel.c > +++ b/ndctl/lib/intel.c > @@ -650,6 +650,29 @@ intel_dimm_cmd_new_lss(struct ndctl_dimm *dimm) > return cmd; > } > > +static bool intel_dimm_fw_update_supported(struct ndctl_dimm *dimm) > +{ > + struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm); > + > + if (!ndctl_dimm_is_cmd_supported(dimm, ND_CMD_CALL)) { > + dbg(ctx, "unsupported cmd: %d\n", ND_CMD_CALL); > + return false; > + } > + > + /* > + * We only need to check FW_GET_INFO. If that isn't supported then > + * the others aren't either. > + */ > + if (test_dimm_dsm(dimm, ND_INTEL_FW_GET_INFO) > + == DIMM_DSM_UNSUPPORTED) { > + dbg(ctx, "unsupported function: %d\n", > + ND_INTEL_FW_GET_INFO); > + return false; > + } > + > + return true; > +} > + > struct ndctl_dimm_ops * const intel_dimm_ops = &(struct ndctl_dimm_ops) { > .cmd_desc = intel_cmd_desc, > .new_smart = intel_dimm_cmd_new_smart, > @@ -703,4 +726,5 @@ struct ndctl_dimm_ops * const intel_dimm_ops = &(struct > ndctl_dimm_ops) { > .fw_fquery_get_fw_rev = intel_cmd_fw_fquery_get_fw_rev, > .fw_xlat_firmware_status = intel_cmd_fw_xlat_firmware_status, > .new_ack_shutdown_count = intel_dimm_cmd_new_lss, > + .fw_update_supported = intel_dimm_fw_update_supported, > }; > diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym > index af9b7d5..cc580f9 100644 > --- a/ndctl/lib/libndctl.sym > +++ b/ndctl/lib/libndctl.sym > @@ -344,4 +344,5 @@ global: > ndctl_cmd_fw_fquery_get_fw_rev; > ndctl_cmd_fw_xlat_firmware_status; > ndctl_dimm_cmd_new_ack_shutdown_count; > + ndctl_dimm_fw_update_supported; > } LIBNDCTL_13; > diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h > index 015eeb2..ae4454c 100644 > --- a/ndctl/lib/private.h > +++ b/ndctl/lib/private.h > @@ -325,6 +325,7 @@ struct ndctl_dimm_ops { > unsigned long long (*fw_fquery_get_fw_rev)(struct ndctl_cmd *); > enum ND_FW_STATUS (*fw_xlat_firmware_status)(struct ndctl_cmd *); > struct ndctl_cmd *(*new_ack_shutdown_count)(struct ndctl_dimm *); > + bool (*fw_update_supported)(struct ndctl_dimm *); > }; > > struct ndctl_dimm_ops * const intel_dimm_ops; > diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h > index 9db775b..08030d3 100644 > --- a/ndctl/libndctl.h > +++ b/ndctl/libndctl.h > @@ -625,6 +625,7 @@ unsigned int ndctl_cmd_fw_start_get_context(struct > ndctl_cmd *cmd); > unsigned long long ndctl_cmd_fw_fquery_get_fw_rev(struct ndctl_cmd *cmd); > enum ND_FW_STATUS ndctl_cmd_fw_xlat_firmware_status(struct ndctl_cmd *cmd); > struct ndctl_cmd *ndctl_dimm_cmd_new_ack_shutdown_count(struct ndctl_dimm > *dimm); > +bool ndctl_dimm_fw_update_supported(struct ndctl_dimm *dimm); > > #ifdef __cplusplus > } /* extern "C" */ > diff --git a/ndctl/update.c b/ndctl/update.c > index 4fb572d..b148b70 100644 > --- a/ndctl/update.c > +++ b/ndctl/update.c > @@ -477,6 +477,10 @@ static int get_ndctl_dimm(struct update_context *uctx, > void *ctx) > 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."); Let's changes this message to: "%s: firmware update not supported.", ndctl_dimm_get_devname(dimm) You don't have enough information to tell if it's the 'kernel' or the 'BIOS', so just say 'not supported'. 'DIMM' is ambiguous, the kernel device name is not. > + return -ENOTSUP; > + } > uctx->dimm = dimm; > rc = update_firmware(uctx); > if (rc < 0) { > @@ -581,7 +585,8 @@ int cmd_update_firmware(int argc, const char **argv, void > *ctx) > > rc = get_ndctl_dimm(&uctx, ctx); > if (rc < 0) { > - error("DIMM %s not found", uctx.dimm_id); > + if (rc == -ENODEV) > + error("DIMM %s not found", uctx.dimm_id); When we move this in with the other dimm functionality in ndctl/dimm.c we should follow the same message convention there and use lowercase 'dimm' and the '"nmem%d", id' format when emitting messages for the user. _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
