On Fri, Jun 9, 2017 at 9:59 AM, Jerry Hoemann <jerry.hoem...@hpe.com> wrote: > On Thu, Jun 08, 2017 at 10:28:51PM -0700, Dan Williams wrote: >> On Thu, Jun 8, 2017 at 1:02 AM, Johannes Thumshirn <jthumsh...@suse.de> >> wrote: >> > On 06/07/2017 07:04 PM, Jerry Hoemann wrote: >> >> @@ -179,6 +217,10 @@ static inline const char >> >> *nvdimm_bus_cmd_name(unsigned cmd) >> >> [ND_CMD_ARS_START] = "ars_start", >> >> [ND_CMD_ARS_STATUS] = "ars_status", >> >> [ND_CMD_CLEAR_ERROR] = "clear_error", >> >> + [5] = "trans_spa", >> >> + [7] = "ars_err_inj", >> >> + [8] = "ars_err_inj_clr", >> >> + [9] = "ars_err_inj_stat", >> >> [ND_CMD_CALL] = "cmd_call", >> >> }; >> > >> > Can you please add the new values to the enum in uapi/ndctl.h? I don't >> > really like the magic numbers here. >> >> I think the reason Jerry didn't add them is due to symmetry. All the >> current ND_CMD_ definitions have corresponding ND_IOCTL_ definitions. >> These new commands we're only adding function number support and using >> ND_CMD_CALL for the ioctl transport. > > Yes, this is correct. > >> We can still add definitions for >> them, but perhaps with an ACPI_NFIT_DSM_ prefix? The only consumer in >> the kernel of this is debug output code. I think perhaps we should >> just delete them here and either print raw numbers or enable >> acpi_nfit_ctl() to the decode and not rely on nvdimm_bus_cmd_name() >> which is meant to represent NVDIMM bus type generic command numbers. > > This also comes into play when doing: > > cat /sys/class/nd/ndctl0/device/nd/ndctl0/device/commands > > Without change here the new functions would show up as "unknown unknown > unknown unknown." > > Dan, are you suggesting I change the above to: > > [ND_CMD_CLEAR_ERROR] = "clear_error", > + [5] = "5", > + [7] = "7", > + [8] = "8", > + [9] = "9", > [ND_CMD_CALL] = "cmd_call", > > Note, "6" is currently reserved by ACPI. I could add that also.
I think it should behave the same as the dimm level 'cmd' vs 'func' distinction. Where only the ND_CMD instances are enumerated in the sysfs '/sys/bus/nd/devices/ndbus0/commands' attribute. The rest of the supported functions are identified in the 'dsm_mask' attribute. We don't currently have one of those for the bus level but we can add it to "/sys/bus/nd/devices/ndbus0/nfit/dsm_mask". This would be similar to "/sys/bus/nd/devices/nmem0/nfit/dsm_mask".
