Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751719AbdFIRNy (ORCPT ); Fri, 9 Jun 2017 13:13:54 -0400 Received: from mail-oi0-f43.google.com ([209.85.218.43]:35700 "EHLO mail-oi0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751664AbdFIRNw (ORCPT ); Fri, 9 Jun 2017 13:13:52 -0400 MIME-Version: 1.0 In-Reply-To: <20170609165931.GA13629@anatevka.americas.hpqcorp.net> References: <1fa949387694a4b6200309d592e78f435f065f75.1496852665.git.jerry.hoemann@hpe.com> <20170609165931.GA13629@anatevka.americas.hpqcorp.net> From: Dan Williams Date: Fri, 9 Jun 2017 10:13:51 -0700 Message-ID: Subject: Re: [PATCH 3/3] libnvdimm: New ACPI 6.2 DSM functions To: Jerry Hoemann Cc: Johannes Thumshirn , "linux-kernel@vger.kernel.org" , "linux-nvdimm@lists.01.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2599 Lines: 56 On Fri, Jun 9, 2017 at 9:59 AM, Jerry Hoemann 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 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".