[ndctl PATCH 0/4] show broken dimm info with translate SPA feature

[ndctl PATCH 0/4] show broken dimm info with translate SPA feature

[Yasunori Goto]

Hello,

I wrote v2 patch set to show broken NVDIMM for ndctl list command.
Please check it.


---
Change log since v1 [1]:
 - Use ND_CMD_CALL to call translate SPA feature.
 - Separate patch set of ndctl from kernel patch set.
 - Add a interface to check what feature can call via ND_CMD_CALL by reading
   /device/nfit/dsm_mask 
 - Get only one nvdimm handle and DPA via ioctl() for the time being.
 - Bug fix which i found
     fix calculation of SPA from bad block at dev_badblocks_to_json().



[1] https://www.mail-archive.com/linux-nvdimm@lists.01.org/msg05287.html


----
This patch set is to show information of broken NVDIMM on ndctl.

When a region has a broken block, user need to replace
the NVDIMM which includes the block.
However there is no information to find which DIMM module have the block.
Not only ndctl does not have such information, nvdimm driver can not
find it.


Fortunately, ACPI 6.2 has new specification of _DSM.
It is "translate spa" which can get NVDIMM handle
and DPA(Dimm Physical Address) from SPA(system Physical Addreess).
It helps for ndctl command to find broken NVDIMM.

This patch set includes followings.
 - some preparations to call Translate SPA via ND_CMD_CALL.
 - libndctl supports Translate SPA interface,
 - ndctl list command show bad DIMM with Translate SPA.


Thanks,
---
Yasunori Goto




_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 1/4] make interface to check device/nfit/dsm_mask flags

[Yasunori Goto]

To check what feature can be called via ND_CMD_CALL, ndctl need to read
device/nfit/dsm_mask. This patch make an interface to check it in libndctl.c


Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 ndctl/lib/libndctl.c | 13 +++++++++++++
 ndctl/libndctl.h.in  |  1 +
 2 files changed, 14 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index c2e0efb..93692e5 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -102,6 +102,7 @@ struct ndctl_bus {
 	size_t buf_len;
 	char *wait_probe_path;
 	unsigned long dsm_mask;
+	unsigned long bus_dsm_mask;
 };
 
 /**
@@ -846,6 +847,12 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
 		bus->revision = strtoul(buf, NULL, 0);
 	}
 
+	sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		bus->bus_dsm_mask = 0;
+	else
+		bus->bus_dsm_mask = strtoul(buf, NULL, 16);
+
 	sprintf(path, "%s/device/provider", ctl_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		goto err_read;
@@ -1101,6 +1108,12 @@ NDCTL_EXPORT int ndctl_bus_is_cmd_supported(struct ndctl_bus *bus,
 	return !!(bus->dsm_mask & (1ULL << cmd));
 }
 
+NDCTL_EXPORT int ndctl_bus_is_sub_cmd_supported(struct ndctl_bus *bus,
+		int cmd)
+{
+	return !!(bus->bus_dsm_mask & (1ULL << cmd));
+}
+
 NDCTL_EXPORT unsigned int ndctl_bus_get_revision(struct ndctl_bus *bus)
 {
 	return bus->revision;
diff --git a/ndctl/libndctl.h.in b/ndctl/libndctl.h.in
index 855d883..206c441 100644
--- a/ndctl/libndctl.h.in
+++ b/ndctl/libndctl.h.in
@@ -109,6 +109,7 @@ struct ndctl_bus *ndctl_bus_get_by_provider(struct ndctl_ctx *ctx,
 		const char *provider);
 const char *ndctl_bus_get_cmd_name(struct ndctl_bus *bus, int cmd);
 int ndctl_bus_is_cmd_supported(struct ndctl_bus *bus, int cmd);
+int ndctl_bus_is_sub_cmd_supported(struct ndctl_bus *bus, int cmd);
 unsigned int ndctl_bus_get_revision(struct ndctl_bus *bus);
 unsigned int ndctl_bus_get_id(struct ndctl_bus *bus);
 const char *ndctl_bus_get_provider(struct ndctl_bus *bus);



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 2/4] allow ND_CMD_CALL for bus

[Yasunori Goto]

Currently ndctl supports ND_CMD_CALL only for DIMM,
but Translate SPA is the feature of bus.
So ND_CMD_CALL must be allowed bus's ioctl().


Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 ndctl/lib/libndctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 93692e5..b3535f0 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2330,6 +2330,7 @@ static int to_ioctl_cmd(int cmd, int dimm)
 #ifdef HAVE_NDCTL_CLEAR_ERROR
 		case ND_CMD_CLEAR_ERROR:     return ND_IOCTL_CLEAR_ERROR;
 #endif
+		case ND_CMD_CALL:            return ND_IOCTL_CALL;
 		default:
 						       return 0;
 		};



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 3/4] Make interfaces to use Translate SPA.

[Yasunori Goto]

This patch makes 3 new interfaces :
  - to ask bus has translate SPA feature.
  - to call translate SPA.
  - to find DIMM by SPA address.


Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
---
 configure.ac                 |  19 ++++++++
 ndctl/lib/libndctl-private.h |   7 +++
 ndctl/lib/libndctl.c         | 111 +++++++++++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym       |   3 ++
 ndctl/libndctl.h.in          |  23 +++++++++
 ndctl/ndctl.h                |   5 ++
 6 files changed, 168 insertions(+)

diff --git a/configure.ac b/configure.ac
index 316f5b7..31d6956 100644
--- a/configure.ac
+++ b/configure.ac
@@ -162,6 +162,25 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
 )
 AM_CONDITIONAL([ENABLE_CLEAR_ERROR], [test "x$enable_clear_err" = "xyes"])
 
+AC_MSG_CHECKING([for TRANSLATE SPA support])
+AC_LANG(C)
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+                       #ifdef HAVE_NDCTL_H
+                       #include <linux/ndctl.h>
+                       #else
+                       #include "ndctl/ndctl.h"
+                       #endif
+                       ]], [[
+                       int x = NFIT_CMD_TRANSLATE_SPA;
+                       ]]
+               )], [AC_MSG_RESULT([yes])
+                    enable_trans_spa=yes
+                    AC_DEFINE([HAVE_NDCTL_TRANS_SPA], [1],
+                               [Define to 1 if ndctl.h has TRANSLATE SPA support.])
+               ], [AC_MSG_RESULT([no])]
+)
+AM_CONDITIONAL([ENABLE_TRANS_SPA], [test "x$enable_trans_spa" = "xyes"])
+
 AC_MSG_CHECKING([for device DAX support])
 AC_LANG(C)
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
diff --git a/ndctl/lib/libndctl-private.h b/ndctl/lib/libndctl-private.h
index 8f10fbc..0512782 100644
--- a/ndctl/lib/libndctl-private.h
+++ b/ndctl/lib/libndctl-private.h
@@ -196,6 +196,7 @@ struct ndctl_cmd {
 #ifdef HAVE_NDCTL_CLEAR_ERROR
 		struct nd_cmd_clear_error clear_err[0];
 #endif
+		struct nd_cmd_trans_spa trans_spa[0];
 		struct ndn_pkg_hpe1 hpe1[0];
 		struct ndn_pkg_msft msft[0];
 		struct nd_cmd_smart smart[0];
@@ -250,6 +251,12 @@ static const int nd_cmd_clear_error = ND_CMD_CLEAR_ERROR;
 static const int nd_cmd_clear_error;
 #endif
 
+#ifdef HAVE_NDCTL_TRANS_SPA
+static const int nd_cmd_trans_spa = NFIT_CMD_TRANSLATE_SPA;
+#else
+static const int nd_cmd_trans_spa;
+#endif
+
 static inline struct ndctl_bus *cmd_to_bus(struct ndctl_cmd *cmd)
 {
 	if (cmd->dimm)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index b3535f0..4556067 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -1959,6 +1959,117 @@ NDCTL_EXPORT struct badblock *ndctl_region_get_first_badblock(struct ndctl_regio
 	return ndctl_region_get_next_badblock(region);
 }
 
+#ifdef HAVE_NDCTL_TRANS_SPA
+NDCTL_EXPORT int ndctl_bus_has_trans_spa(struct ndctl_bus *bus)
+{
+	if (!bus)
+		return 0;
+
+	return ndctl_bus_is_sub_cmd_supported(bus, NFIT_CMD_TRANSLATE_SPA);
+}
+
+static struct ndctl_cmd *ndctl_bus_cmd_new_trans_spa(struct ndctl_bus *bus)
+{
+	struct ndctl_cmd *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nd_cmd_trans_spa *trans_spa;
+	size_t size, spa_length;
+
+	spa_length = sizeof(struct nd_cmd_trans_spa)
+		+ sizeof(struct nd_nvdimm_device);
+	size = sizeof(*cmd) + sizeof(*pkg) + spa_length;
+	cmd = calloc(1, size);
+	if (!cmd)
+		return NULL;
+
+	cmd->bus = bus;
+	ndctl_cmd_ref(cmd);
+	cmd->type = ND_CMD_CALL;
+	cmd->size = size;
+	cmd->status = 1;
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	pkg->nd_command = NFIT_CMD_TRANSLATE_SPA;
+	pkg->nd_size_in = sizeof(unsigned long long);
+	pkg->nd_size_out = spa_length;
+	pkg->nd_fw_size = spa_length;
+	trans_spa = (struct nd_cmd_trans_spa *)&pkg->nd_payload[0];
+	cmd->firmware_status = &trans_spa->status;
+	trans_spa->trans_length = spa_length;
+
+	return cmd;
+}
+
+static int ndctl_bus_cmd_get_trans_spa(struct ndctl_cmd *cmd,
+					unsigned int *handle, unsigned long long *dpa)
+{
+	struct nd_cmd_pkg *pkg;
+	struct nd_cmd_trans_spa *trans_spa;
+
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	trans_spa = (struct nd_cmd_trans_spa *)&pkg->nd_payload[0];
+
+	if (trans_spa->status == ND_TRANS_SPA_STATUS_INVALID_SPA)
+		return -EINVAL;
+
+	/*
+	 * XXX: Currently NVDIMM mirroring is not supported.
+	 * Even if ACPI returned plural dimms due to mirroring,
+	 * this function returns just the first dimm.
+	 */
+
+	*handle = trans_spa->devices[0].nfit_device_handle;
+	*dpa = trans_spa->devices[0].dpa;
+
+	return 0;
+}
+
+NDCTL_EXPORT int ndctl_bus_cmd_trans_spa(struct ndctl_bus *bus,
+	unsigned long long addr, unsigned int *handle, unsigned long long *dpa)
+{
+
+	struct ndctl_cmd *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nd_cmd_trans_spa *trans_spa;
+	int rc;
+
+	cmd = ndctl_bus_cmd_new_trans_spa(bus);
+	if (!cmd)
+		return -ENOMEM;
+
+	pkg = (struct nd_cmd_pkg *)&cmd->cmd_buf[0];
+	trans_spa = (struct nd_cmd_trans_spa *)&pkg->nd_payload[0];
+	trans_spa->spa = addr;
+
+	rc = ndctl_cmd_submit(cmd);
+	if (rc) {
+		ndctl_cmd_unref(cmd);
+		return rc;
+	}
+
+	rc = ndctl_bus_cmd_get_trans_spa(cmd, handle, dpa);
+	ndctl_cmd_unref(cmd);
+
+	return rc;
+}
+
+NDCTL_EXPORT struct ndctl_dimm *ndctl_dimm_get_by_spa(struct ndctl_bus *bus,
+		unsigned long long spa)
+{
+	int rc;
+	unsigned int handle;
+	unsigned long long dpa;
+
+	if (!bus || !spa)
+		return NULL;
+
+	rc = ndctl_bus_cmd_trans_spa(bus, spa, &handle, &dpa);
+	if (rc)
+		return NULL;
+
+	return ndctl_dimm_get_by_handle(bus, handle);
+}
+#endif /* HAVE_NDCTL_TRANS_SPA */
+
 static struct nd_cmd_vendor_tail *to_vendor_tail(struct ndctl_cmd *cmd)
 {
 	struct nd_cmd_vendor_tail *tail = (struct nd_cmd_vendor_tail *)
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index b8ac65f..8d9b272 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -36,6 +36,9 @@ global:
 	ndctl_bus_get_provider;
 	ndctl_bus_get_ctx;
 	ndctl_bus_wait_probe;
+	ndctl_bus_has_trans_spa;
+	ndctl_bus_cmd_trans_spa;
+	ndctl_dimm_get_by_spa;
 	ndctl_dimm_get_first;
 	ndctl_dimm_get_next;
 	ndctl_dimm_get_handle;
diff --git a/ndctl/libndctl.h.in b/ndctl/libndctl.h.in
index 206c441..f85a8e7 100644
--- a/ndctl/libndctl.h.in
+++ b/ndctl/libndctl.h.in
@@ -348,6 +348,29 @@ static inline unsigned int ndctl_cmd_smart_threshold_get_spares(
 }
 #endif
 
+#if HAVE_NDCTL_TRANS_SPA == 1
+int ndctl_bus_has_trans_spa(struct ndctl_bus *bus);
+int ndctl_bus_cmd_trans_spa(struct ndctl_bus *bus,
+	unsigned long long addr, unsigned int *handle, unsigned long long *dpa);
+struct ndctl_dimm *ndctl_dimm_get_by_spa(struct ndctl_bus *bus,
+	unsigned long long spa);
+#else
+static inline int ndctl_bus_has_trans_spa(struct ndctl_bus *bus)
+{
+	return 0;
+}
+static inline int ndctl_bus_cmd_trans_spa(struct ndctl_bus *bus,
+	unsigned long long addr, unsigned int *handle, unsigned long long *dpa)
+{
+	return 0;
+}
+static inline ndctl_dimm *ndctl_dimm_get_by_spa(struct ndctl_bus *bus,
+	unsigned long long spa)
+{
+	return NULL;
+}
+#endif
+
 struct ndctl_cmd *ndctl_dimm_cmd_new_vendor_specific(struct ndctl_dimm *dimm,
 		unsigned int opcode, size_t input_size, size_t output_size);
 ssize_t ndctl_cmd_vendor_set_input(struct ndctl_cmd *cmd, void *buf,
diff --git a/ndctl/ndctl.h b/ndctl/ndctl.h
index d70b97d..0efdb3d 100644
--- a/ndctl/ndctl.h
+++ b/ndctl/ndctl.h
@@ -35,6 +35,8 @@ struct nd_cmd_smart {
 #define ND_SMART_CRITICAL_HEALTH	(1 << 1)
 #define ND_SMART_FATAL_HEALTH		(1 << 2)
 
+#define ND_TRANS_SPA_STATUS_INVALID_SPA  2
+
 struct nd_smart_payload {
 	__u32 flags;
 	__u8 reserved0[4];
@@ -191,6 +193,9 @@ enum {
 	ND_CMD_ARS_STATUS = 3,
 	ND_CMD_CLEAR_ERROR = 4,
 
+	/* bus sub commands */
+	NFIT_CMD_TRANSLATE_SPA = 5,
+
 	/* per-dimm commands */
 	ND_CMD_SMART = 1,
 	ND_CMD_SMART_THRESHOLD = 2,



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 4/4] show bad dimm's name by ndctl list command.

[Yasunori Goto]

Show dimm's name which has badblock by ndctl list command.
This patch uses translate SPA interface to get bad dimm info.


Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>

---
 util/json.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/util/json.c b/util/json.c
index 98165b7..b9ab420 100644
--- a/util/json.c
+++ b/util/json.c
@@ -366,6 +366,18 @@ struct json_object *util_daxctl_region_to_json(struct daxctl_region *region,
 	return NULL;
 }
 
+static struct ndctl_dimm *badblock_to_dimm(struct ndctl_region *region,
+		unsigned long long spa)
+{
+	struct ndctl_bus *bus;
+
+	bus = ndctl_region_get_bus(region);
+	if (!ndctl_bus_has_trans_spa(bus))
+		return NULL;
+
+	return ndctl_dimm_get_by_spa(bus, spa);
+}
+
 struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 		unsigned int *bb_count, unsigned long flags)
 {
@@ -381,6 +393,18 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 
 	ndctl_region_badblock_foreach(region, bb) {
 		if (flags & UTIL_JSON_MEDIA_ERRORS) {
+			struct ndctl_dimm *dimm = NULL;
+			unsigned long long spa;
+
+			/* get start address of region */
+			spa = ndctl_region_get_resource(region);
+			if (spa == ULONG_MAX)
+				goto err_array;
+
+			/* get address of bad block */
+			spa += bb->offset << 9;
+			dimm = badblock_to_dimm(region, spa);
+
 			jbb = json_object_new_object();
 			if (!jbb)
 				goto err_array;
@@ -395,6 +419,12 @@ struct json_object *util_region_badblocks_to_json(struct ndctl_region *region,
 				goto err;
 			json_object_object_add(jbb, "length", jobj);
 
+			if (dimm) {
+				jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
+				if (!jobj)
+					goto err;
+				json_object_object_add(jbb, "dimm", jobj);
+			}
 			json_object_array_add(jbbs, jbb);
 		}
 
@@ -436,6 +466,7 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 
 	ndctl_region_badblock_foreach(region, bb) {
 		unsigned long long bb_begin, bb_end, begin, end;
+		struct ndctl_dimm *dimm = NULL;
 
 		bb_begin = region_begin + (bb->offset << 9);
 		bb_end = bb_begin + (bb->len << 9) - 1;
@@ -456,6 +487,8 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 		offset = (begin - dev_begin) >> 9;
 		len = (end - begin + 1) >> 9;
 
+		dimm = badblock_to_dimm(region, begin);
+
 		if (flags & UTIL_JSON_MEDIA_ERRORS) {
 			/* add to json */
 			jbb = json_object_new_object();
@@ -472,6 +505,13 @@ static struct json_object *dev_badblocks_to_json(struct ndctl_region *region,
 				goto err;
 			json_object_object_add(jbb, "length", jobj);
 
+			if (dimm) {
+				jobj = json_object_new_string(ndctl_dimm_get_devname(dimm));
+				if (!jobj)
+					goto err;
+				json_object_object_add(jbb, "dimm", jobj);
+			}
+
 			json_object_array_add(jbbs, jbb);
 		}
 		bbs += len;



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 1/4] make interface to check device/nfit/dsm_mask flags

[Dan Williams]

On Thu, Aug 17, 2017 at 9:33 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>
> To check what feature can be called via ND_CMD_CALL, ndctl need to read
> device/nfit/dsm_mask. This patch make an interface to check it in libndctl.c
>
>
> Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> ---
>  ndctl/lib/libndctl.c | 13 +++++++++++++
>  ndctl/libndctl.h.in  |  1 +
>  2 files changed, 14 insertions(+)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index c2e0efb..93692e5 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -102,6 +102,7 @@ struct ndctl_bus {
>         size_t buf_len;
>         char *wait_probe_path;
>         unsigned long dsm_mask;
> +       unsigned long bus_dsm_mask;
>  };
>
>  /**
> @@ -846,6 +847,12 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
>                 bus->revision = strtoul(buf, NULL, 0);
>         }
>
> +       sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
> +       if (sysfs_read_attr(ctx, path, buf) < 0)
> +               bus->bus_dsm_mask = 0;
> +       else
> +               bus->bus_dsm_mask = strtoul(buf, NULL, 16);
> +
>         sprintf(path, "%s/device/provider", ctl_base);
>         if (sysfs_read_attr(ctx, path, buf) < 0)
>                 goto err_read;
> @@ -1101,6 +1108,12 @@ NDCTL_EXPORT int ndctl_bus_is_cmd_supported(struct ndctl_bus *bus,
>         return !!(bus->dsm_mask & (1ULL << cmd));
>  }
>
> +NDCTL_EXPORT int ndctl_bus_is_sub_cmd_supported(struct ndctl_bus *bus,

Looks good, let's call this ndctl_bus_is_passthru_cmd_supported()
since ndctl_bus_is_cmd_supported() is telling you about native bus
commands that Linux supports generically and "passthru" is for command
payloads that Linux is just blindly passing through to the firmware
implementation. As far as the libndctl user is concerned they should
not be able to easily tell the difference between an ACPI defined
NVDIMM bus or Open Firmware / some other platform.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 2/4] allow ND_CMD_CALL for bus

[Dan Williams]

On Thu, Aug 17, 2017 at 9:34 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>
> Currently ndctl supports ND_CMD_CALL only for DIMM,
> but Translate SPA is the feature of bus.
> So ND_CMD_CALL must be allowed bus's ioctl().
>
>
> Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> ---
>  ndctl/lib/libndctl.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 93692e5..b3535f0 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -2330,6 +2330,7 @@ static int to_ioctl_cmd(int cmd, int dimm)
>  #ifdef HAVE_NDCTL_CLEAR_ERROR
>                 case ND_CMD_CLEAR_ERROR:     return ND_IOCTL_CLEAR_ERROR;
>  #endif
> +               case ND_CMD_CALL:            return ND_IOCTL_CALL;
>                 default:
>                                                        return 0;
>                 };
>

Looks good.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 3/4] Make interfaces to use Translate SPA.

[Dan Williams]

On Thu, Aug 17, 2017 at 9:35 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>
> This patch makes 3 new interfaces :
>   - to ask bus has translate SPA feature.
>   - to call translate SPA.
>   - to find DIMM by SPA address.
>
>
> Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> ---
>  configure.ac                 |  19 ++++++++
>  ndctl/lib/libndctl-private.h |   7 +++
>  ndctl/lib/libndctl.c         | 111 +++++++++++++++++++++++++++++++++++++++++++
>  ndctl/lib/libndctl.sym       |   3 ++
>  ndctl/libndctl.h.in          |  23 +++++++++
>  ndctl/ndctl.h                |   5 ++
>  6 files changed, 168 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index 316f5b7..31d6956 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -162,6 +162,25 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
>  )
>  AM_CONDITIONAL([ENABLE_CLEAR_ERROR], [test "x$enable_clear_err" = "xyes"])
>
> +AC_MSG_CHECKING([for TRANSLATE SPA support])
> +AC_LANG(C)
> +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
> +                       #ifdef HAVE_NDCTL_H
> +                       #include <linux/ndctl.h>
> +                       #else
> +                       #include "ndctl/ndctl.h"
> +                       #endif
> +                       ]], [[
> +                       int x = NFIT_CMD_TRANSLATE_SPA;
> +                       ]]
> +               )], [AC_MSG_RESULT([yes])
> +                    enable_trans_spa=yes
> +                    AC_DEFINE([HAVE_NDCTL_TRANS_SPA], [1],
> +                               [Define to 1 if ndctl.h has TRANSLATE SPA support.])
> +               ], [AC_MSG_RESULT([no])]
> +)
> +AM_CONDITIONAL([ENABLE_TRANS_SPA], [test "x$enable_trans_spa" = "xyes"])

Since the kernel is not defining a custom ioctl for this I don't think
I want to use ndctl.h to carry the definition of
NFIT_CMD_TRANSLATE_SPA. Instead I think we should create a
libndctl-nfit.h header that gets installed alongside libndctl.h. Then
consumers can use ndctl_bus_has_nfit() to check that they are talking
to an NFIT-defined NVDIMM bus and use the NFIT-specific commands
defined in that header file.

> +
>  AC_MSG_CHECKING([for device DAX support])
>  AC_LANG(C)
>  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
> diff --git a/ndctl/lib/libndctl-private.h b/ndctl/lib/libndctl-private.h
> index 8f10fbc..0512782 100644
> --- a/ndctl/lib/libndctl-private.h
> +++ b/ndctl/lib/libndctl-private.h
> @@ -196,6 +196,7 @@ struct ndctl_cmd {
>  #ifdef HAVE_NDCTL_CLEAR_ERROR
>                 struct nd_cmd_clear_error clear_err[0];
>  #endif
> +               struct nd_cmd_trans_spa trans_spa[0];
>                 struct ndn_pkg_hpe1 hpe1[0];
>                 struct ndn_pkg_msft msft[0];
>                 struct nd_cmd_smart smart[0];
> @@ -250,6 +251,12 @@ static const int nd_cmd_clear_error = ND_CMD_CLEAR_ERROR;
>  static const int nd_cmd_clear_error;
>  #endif
>
> +#ifdef HAVE_NDCTL_TRANS_SPA
> +static const int nd_cmd_trans_spa = NFIT_CMD_TRANSLATE_SPA;
> +#else
> +static const int nd_cmd_trans_spa;
> +#endif
> +
>  static inline struct ndctl_bus *cmd_to_bus(struct ndctl_cmd *cmd)
>  {
>         if (cmd->dimm)
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index b3535f0..4556067 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -1959,6 +1959,117 @@ NDCTL_EXPORT struct badblock *ndctl_region_get_first_badblock(struct ndctl_regio
>         return ndctl_region_get_next_badblock(region);
>  }

I believe these functions below will always be NFIT specific so lets
move them to a new ndctl/lib/libndctl-nfit.c file.

>
> +#ifdef HAVE_NDCTL_TRANS_SPA
> +NDCTL_EXPORT int ndctl_bus_has_trans_spa(struct ndctl_bus *bus)

Hmm, now that I read this patch perhaps we should just have this local
capability check and drop the generic
ndctl_bus_is_passthru_cmd_supported() helper since the only reason to
check for sub / passthru command is if you know you are talking to an
NFIT bus.

> +{
> +       if (!bus)
> +               return 0;

All these functions should also have

    if (!ndctl_bus_has_nfit(bus))
        return false / NULL / etc...


Another general comment is to rename trans_spa to translate_spa, the
function name is already long, so no need to abbreviate.

Other than those comments this looks good to me.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 3/4] Make interfaces to use Translate SPA.

[Dan Williams]

On Thu, Aug 17, 2017 at 9:35 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>
> This patch makes 3 new interfaces :
>   - to ask bus has translate SPA feature.
>   - to call translate SPA.
>   - to find DIMM by SPA address.
>
>
> Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> ---
>  configure.ac                 |  19 ++++++++
>  ndctl/lib/libndctl-private.h |   7 +++
>  ndctl/lib/libndctl.c         | 111 +++++++++++++++++++++++++++++++++++++++++++
>  ndctl/lib/libndctl.sym       |   3 ++
>  ndctl/libndctl.h.in          |  23 +++++++++
>  ndctl/ndctl.h                |   5 ++
>  6 files changed, 168 insertions(+)
>
[..]
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index b3535f0..4556067 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
[..]

Sorry I overlooked this function in my last mail:

> +NDCTL_EXPORT struct ndctl_dimm *ndctl_dimm_get_by_spa(struct ndctl_bus *bus,
> +               unsigned long long spa)
> +{
> +       int rc;
> +       unsigned int handle;
> +       unsigned long long dpa;
> +
> +       if (!bus || !spa)
> +               return NULL;

We can do some sanity checking here to check the resource range and
see if the DIMM is in that region. See ndctl_region_get_resource() and
ndctl_dimm_foreach_in_region(). That also allows us to support single
dimm regions without calling translate spa.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 1/4] make interface to check device/nfit/dsm_mask flags

[Yasunori Goto]

> On Thu, Aug 17, 2017 at 9:33 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> >
> > To check what feature can be called via ND_CMD_CALL, ndctl need to read
> > device/nfit/dsm_mask. This patch make an interface to check it in libndctl.c
> >
> >
> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> > ---
> >  ndctl/lib/libndctl.c | 13 +++++++++++++
> >  ndctl/libndctl.h.in  |  1 +
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > index c2e0efb..93692e5 100644
> > --- a/ndctl/lib/libndctl.c
> > +++ b/ndctl/lib/libndctl.c
> > @@ -102,6 +102,7 @@ struct ndctl_bus {
> >         size_t buf_len;
> >         char *wait_probe_path;
> >         unsigned long dsm_mask;
> > +       unsigned long bus_dsm_mask;
> >  };
> >
> >  /**
> > @@ -846,6 +847,12 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
> >                 bus->revision = strtoul(buf, NULL, 0);
> >         }
> >
> > +       sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
> > +       if (sysfs_read_attr(ctx, path, buf) < 0)
> > +               bus->bus_dsm_mask = 0;
> > +       else
> > +               bus->bus_dsm_mask = strtoul(buf, NULL, 16);
> > +
> >         sprintf(path, "%s/device/provider", ctl_base);
> >         if (sysfs_read_attr(ctx, path, buf) < 0)
> >                 goto err_read;
> > @@ -1101,6 +1108,12 @@ NDCTL_EXPORT int ndctl_bus_is_cmd_supported(struct ndctl_bus *bus,
> >         return !!(bus->dsm_mask & (1ULL << cmd));
> >  }
> >
> > +NDCTL_EXPORT int ndctl_bus_is_sub_cmd_supported(struct ndctl_bus *bus,
> 
> Looks good, let's call this ndctl_bus_is_passthru_cmd_supported()
> since ndctl_bus_is_cmd_supported() is telling you about native bus
> commands that Linux supports generically and "passthru" is for command
> payloads that Linux is just blindly passing through to the firmware
> implementation. As far as the libndctl user is concerned they should
> not be able to easily tell the difference between an ACPI defined
> NVDIMM bus or Open Firmware / some other platform.
> 

I see. I'll change it.



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 3/4] Make interfaces to use Translate SPA.

[Yasunori Goto]

> On Thu, Aug 17, 2017 at 9:35 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> >
> > This patch makes 3 new interfaces :
> >   - to ask bus has translate SPA feature.
> >   - to call translate SPA.
> >   - to find DIMM by SPA address.
> >
> >
> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> > ---
> >  configure.ac                 |  19 ++++++++
> >  ndctl/lib/libndctl-private.h |   7 +++
> >  ndctl/lib/libndctl.c         | 111 +++++++++++++++++++++++++++++++++++++++++++
> >  ndctl/lib/libndctl.sym       |   3 ++
> >  ndctl/libndctl.h.in          |  23 +++++++++
> >  ndctl/ndctl.h                |   5 ++
> >  6 files changed, 168 insertions(+)
> >
> [..]
> > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > index b3535f0..4556067 100644
> > --- a/ndctl/lib/libndctl.c
> > +++ b/ndctl/lib/libndctl.c
> [..]
> 
> Sorry I overlooked this function in my last mail:
> 
> > +NDCTL_EXPORT struct ndctl_dimm *ndctl_dimm_get_by_spa(struct ndctl_bus *bus,
> > +               unsigned long long spa)
> > +{
> > +       int rc;
> > +       unsigned int handle;
> > +       unsigned long long dpa;
> > +
> > +       if (!bus || !spa)
> > +               return NULL;
> 
> We can do some sanity checking here to check the resource range and
> see if the DIMM is in that region. See ndctl_region_get_resource() and
> ndctl_dimm_foreach_in_region(). That also allows us to support single
> dimm regions without calling translate spa.
> 

Ok. I'll check it.



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 3/4] Make interfaces to use Translate SPA.

[Yasunori Goto]

> On Thu, Aug 17, 2017 at 9:35 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
> >
> > This patch makes 3 new interfaces :
> >   - to ask bus has translate SPA feature.
> >   - to call translate SPA.
> >   - to find DIMM by SPA address.
> >
> >
> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
> > ---
> >  configure.ac                 |  19 ++++++++
> >  ndctl/lib/libndctl-private.h |   7 +++
> >  ndctl/lib/libndctl.c         | 111 +++++++++++++++++++++++++++++++++++++++++++
> >  ndctl/lib/libndctl.sym       |   3 ++
> >  ndctl/libndctl.h.in          |  23 +++++++++
> >  ndctl/ndctl.h                |   5 ++
> >  6 files changed, 168 insertions(+)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 316f5b7..31d6956 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -162,6 +162,25 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
> >  )
> >  AM_CONDITIONAL([ENABLE_CLEAR_ERROR], [test "x$enable_clear_err" = "xyes"])
> >
> > +AC_MSG_CHECKING([for TRANSLATE SPA support])
> > +AC_LANG(C)
> > +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
> > +                       #ifdef HAVE_NDCTL_H
> > +                       #include <linux/ndctl.h>
> > +                       #else
> > +                       #include "ndctl/ndctl.h"
> > +                       #endif
> > +                       ]], [[
> > +                       int x = NFIT_CMD_TRANSLATE_SPA;
> > +                       ]]
> > +               )], [AC_MSG_RESULT([yes])
> > +                    enable_trans_spa=yes
> > +                    AC_DEFINE([HAVE_NDCTL_TRANS_SPA], [1],
> > +                               [Define to 1 if ndctl.h has TRANSLATE SPA support.])
> > +               ], [AC_MSG_RESULT([no])]
> > +)
> > +AM_CONDITIONAL([ENABLE_TRANS_SPA], [test "x$enable_trans_spa" = "xyes"])
> 
> Since the kernel is not defining a custom ioctl for this I don't think
> I want to use ndctl.h to carry the definition of
> NFIT_CMD_TRANSLATE_SPA. Instead I think we should create a
> libndctl-nfit.h header that gets installed alongside libndctl.h. Then
> consumers can use ndctl_bus_has_nfit() to check that they are talking
> to an NFIT-defined NVDIMM bus and use the NFIT-specific commands
> defined in that header file.


Ok.

> 
> > +
> >  AC_MSG_CHECKING([for device DAX support])
> >  AC_LANG(C)
> >  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
> > diff --git a/ndctl/lib/libndctl-private.h b/ndctl/lib/libndctl-private.h
> > index 8f10fbc..0512782 100644
> > --- a/ndctl/lib/libndctl-private.h
> > +++ b/ndctl/lib/libndctl-private.h
> > @@ -196,6 +196,7 @@ struct ndctl_cmd {
> >  #ifdef HAVE_NDCTL_CLEAR_ERROR
> >                 struct nd_cmd_clear_error clear_err[0];
> >  #endif
> > +               struct nd_cmd_trans_spa trans_spa[0];
> >                 struct ndn_pkg_hpe1 hpe1[0];
> >                 struct ndn_pkg_msft msft[0];
> >                 struct nd_cmd_smart smart[0];
> > @@ -250,6 +251,12 @@ static const int nd_cmd_clear_error = ND_CMD_CLEAR_ERROR;
> >  static const int nd_cmd_clear_error;
> >  #endif
> >
> > +#ifdef HAVE_NDCTL_TRANS_SPA
> > +static const int nd_cmd_trans_spa = NFIT_CMD_TRANSLATE_SPA;
> > +#else
> > +static const int nd_cmd_trans_spa;
> > +#endif
> > +
> >  static inline struct ndctl_bus *cmd_to_bus(struct ndctl_cmd *cmd)
> >  {
> >         if (cmd->dimm)
> > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> > index b3535f0..4556067 100644
> > --- a/ndctl/lib/libndctl.c
> > +++ b/ndctl/lib/libndctl.c
> > @@ -1959,6 +1959,117 @@ NDCTL_EXPORT struct badblock *ndctl_region_get_first_badblock(struct ndctl_regio
> >         return ndctl_region_get_next_badblock(region);
> >  }
> 
> I believe these functions below will always be NFIT specific so lets
> move them to a new ndctl/lib/libndctl-nfit.c file.
> 
> >
> > +#ifdef HAVE_NDCTL_TRANS_SPA
> > +NDCTL_EXPORT int ndctl_bus_has_trans_spa(struct ndctl_bus *bus)
> 
> Hmm, now that I read this patch perhaps we should just have this local
> capability check and drop the generic
> ndctl_bus_is_passthru_cmd_supported() helper since the only reason to
> check for sub / passthru command is if you know you are talking to an
> NFIT bus.
> 
> > +{
> > +       if (!bus)
> > +               return 0;
> 
> All these functions should also have
> 
>     if (!ndctl_bus_has_nfit(bus))
>         return false / NULL / etc...
> 
> 
> Another general comment is to rename trans_spa to translate_spa, the
> function name is already long, so no need to abbreviate.

Hmm.

I found "struct nd_cmd_trans_spa" is already defined in 
ndctl/ndctl.h of ndctl tree, and include/uapi/linux/ndctl.h of kernel tree.

Do you think it should be changed too?
If yes, I'll do.


> 
> Other than those comments this looks good to me.

Thanks for your review!



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 3/4] Make interfaces to use Translate SPA.

[Dan Williams]

On Sun, Aug 27, 2017 at 6:43 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>> On Thu, Aug 17, 2017 at 9:35 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote:
>> >
>> > This patch makes 3 new interfaces :
>> >   - to ask bus has translate SPA feature.
>> >   - to call translate SPA.
>> >   - to find DIMM by SPA address.
>> >
>> >
>> > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com>
>> > ---
>> >  configure.ac                 |  19 ++++++++
>> >  ndctl/lib/libndctl-private.h |   7 +++
>> >  ndctl/lib/libndctl.c         | 111 +++++++++++++++++++++++++++++++++++++++++++
>> >  ndctl/lib/libndctl.sym       |   3 ++
>> >  ndctl/libndctl.h.in          |  23 +++++++++
>> >  ndctl/ndctl.h                |   5 ++
>> >  6 files changed, 168 insertions(+)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index 316f5b7..31d6956 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -162,6 +162,25 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
>> >  )
>> >  AM_CONDITIONAL([ENABLE_CLEAR_ERROR], [test "x$enable_clear_err" = "xyes"])
>> >
>> > +AC_MSG_CHECKING([for TRANSLATE SPA support])
>> > +AC_LANG(C)
>> > +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
>> > +                       #ifdef HAVE_NDCTL_H
>> > +                       #include <linux/ndctl.h>
>> > +                       #else
>> > +                       #include "ndctl/ndctl.h"
>> > +                       #endif
>> > +                       ]], [[
>> > +                       int x = NFIT_CMD_TRANSLATE_SPA;
>> > +                       ]]
>> > +               )], [AC_MSG_RESULT([yes])
>> > +                    enable_trans_spa=yes
>> > +                    AC_DEFINE([HAVE_NDCTL_TRANS_SPA], [1],
>> > +                               [Define to 1 if ndctl.h has TRANSLATE SPA support.])
>> > +               ], [AC_MSG_RESULT([no])]
>> > +)
>> > +AM_CONDITIONAL([ENABLE_TRANS_SPA], [test "x$enable_trans_spa" = "xyes"])
>>
>> Since the kernel is not defining a custom ioctl for this I don't think
>> I want to use ndctl.h to carry the definition of
>> NFIT_CMD_TRANSLATE_SPA. Instead I think we should create a
>> libndctl-nfit.h header that gets installed alongside libndctl.h. Then
>> consumers can use ndctl_bus_has_nfit() to check that they are talking
>> to an NFIT-defined NVDIMM bus and use the NFIT-specific commands
>> defined in that header file.
>
>
> Ok.
>
>>
>> > +
>> >  AC_MSG_CHECKING([for device DAX support])
>> >  AC_LANG(C)
>> >  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
>> > diff --git a/ndctl/lib/libndctl-private.h b/ndctl/lib/libndctl-private.h
>> > index 8f10fbc..0512782 100644
>> > --- a/ndctl/lib/libndctl-private.h
>> > +++ b/ndctl/lib/libndctl-private.h
>> > @@ -196,6 +196,7 @@ struct ndctl_cmd {
>> >  #ifdef HAVE_NDCTL_CLEAR_ERROR
>> >                 struct nd_cmd_clear_error clear_err[0];
>> >  #endif
>> > +               struct nd_cmd_trans_spa trans_spa[0];
>> >                 struct ndn_pkg_hpe1 hpe1[0];
>> >                 struct ndn_pkg_msft msft[0];
>> >                 struct nd_cmd_smart smart[0];
>> > @@ -250,6 +251,12 @@ static const int nd_cmd_clear_error = ND_CMD_CLEAR_ERROR;
>> >  static const int nd_cmd_clear_error;
>> >  #endif
>> >
>> > +#ifdef HAVE_NDCTL_TRANS_SPA
>> > +static const int nd_cmd_trans_spa = NFIT_CMD_TRANSLATE_SPA;
>> > +#else
>> > +static const int nd_cmd_trans_spa;
>> > +#endif
>> > +
>> >  static inline struct ndctl_bus *cmd_to_bus(struct ndctl_cmd *cmd)
>> >  {
>> >         if (cmd->dimm)
>> > diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
>> > index b3535f0..4556067 100644
>> > --- a/ndctl/lib/libndctl.c
>> > +++ b/ndctl/lib/libndctl.c
>> > @@ -1959,6 +1959,117 @@ NDCTL_EXPORT struct badblock *ndctl_region_get_first_badblock(struct ndctl_regio
>> >         return ndctl_region_get_next_badblock(region);
>> >  }
>>
>> I believe these functions below will always be NFIT specific so lets
>> move them to a new ndctl/lib/libndctl-nfit.c file.
>>
>> >
>> > +#ifdef HAVE_NDCTL_TRANS_SPA
>> > +NDCTL_EXPORT int ndctl_bus_has_trans_spa(struct ndctl_bus *bus)
>>
>> Hmm, now that I read this patch perhaps we should just have this local
>> capability check and drop the generic
>> ndctl_bus_is_passthru_cmd_supported() helper since the only reason to
>> check for sub / passthru command is if you know you are talking to an
>> NFIT bus.
>>
>> > +{
>> > +       if (!bus)
>> > +               return 0;
>>
>> All these functions should also have
>>
>>     if (!ndctl_bus_has_nfit(bus))
>>         return false / NULL / etc...
>>
>>
>> Another general comment is to rename trans_spa to translate_spa, the
>> function name is already long, so no need to abbreviate.
>
> Hmm.
>
> I found "struct nd_cmd_trans_spa" is already defined in
> ndctl/ndctl.h of ndctl tree, and include/uapi/linux/ndctl.h of kernel tree.
>
> Do you think it should be changed too?
> If yes, I'll do.

Yes, if we have libndctl-nfit.h we don't need those defines in the
kernel. We should remove them before 4.13 final.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm