Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751798AbdFIQ7e (ORCPT ); Fri, 9 Jun 2017 12:59:34 -0400 Received: from g2t2353.austin.hpe.com ([15.233.44.26]:42553 "EHLO g2t2353.austin.hpe.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751578AbdFIQ7c (ORCPT ); Fri, 9 Jun 2017 12:59:32 -0400 X-Greylist: delayed 172472 seconds by postgrey-1.27 at vger.kernel.org; Fri, 09 Jun 2017 12:59:32 EDT Date: Fri, 9 Jun 2017 10:59:31 -0600 From: Jerry Hoemann To: Dan Williams Cc: Johannes Thumshirn , "linux-kernel@vger.kernel.org" , "linux-nvdimm@lists.01.org" Subject: Re: [PATCH 3/3] libnvdimm: New ACPI 6.2 DSM functions Message-ID: <20170609165931.GA13629@anatevka.americas.hpqcorp.net> Reply-To: Jerry.Hoemann@hpe.com References: <1fa949387694a4b6200309d592e78f435f065f75.1496852665.git.jerry.hoemann@hpe.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2206 Lines: 55 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. -- ----------------------------------------------------------------------------- Jerry Hoemann Software Engineer Hewlett Packard Enterprise -----------------------------------------------------------------------------