[PATCH] nvme: Elaborate on cache maintenance operation in get/set_features

[Andre Przywara]

At the moment the nvme_get_features() and nvme_set_features() functions
carry a (somewhat misleading) comment about missing cache maintenance.

As it turns out, nvme_get_features() has no caller at all in the tree,
and nvme_set_features' only user doesn't use a DMA buffer.

Mention that in the comment, and leave some breadcrumbs for the future,
should those functions attract more users.

Signed-off-by: Andre Przywara <andre.przywara at arm.com>
---
Hi,

just extending the comments as this caused some confusion lately, and to
keep the pointer to the NVMe spec I checked.

Cheers,
Andre

 drivers/nvme/nvme.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 5d6331ad346..3ff3d5de08e 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -481,6 +481,7 @@ int nvme_get_features(struct nvme_dev *dev, unsigned fid, unsigned nsid,
 		      dma_addr_t dma_addr, u32 *result)
 {
 	struct nvme_command c;
+	int ret;
 
 	memset(&c, 0, sizeof(c));
 	c.features.opcode = nvme_admin_get_features;
@@ -488,12 +489,20 @@ int nvme_get_features(struct nvme_dev *dev, unsigned fid, unsigned nsid,
 	c.features.prp1 = cpu_to_le64(dma_addr);
 	c.features.fid = cpu_to_le32(fid);
 
+	ret = nvme_submit_admin_cmd(dev, &c, result);
+
 	/*
-	 * TODO: add cache invalidate operation when the size of
-	 * the DMA buffer is known
+	 * TODO: Add some cache invalidation when a DMA buffer is involved
+	 * in the request, here and before the command gets submitted. The
+	 * buffer size varies by feature, also some features use a different
+	 * field in the command packet to hold the buffer address.
+	 * Section 5.21.1 (Set Features command) in the NVMe specification
+	 * details the buffer requirements for each feature.
+	 *
+	 * At the moment there is no user of this function.
 	 */
 
-	return nvme_submit_admin_cmd(dev, &c, result);
+	return ret;
 }
 
 int nvme_set_features(struct nvme_dev *dev, unsigned fid, unsigned dword11,
@@ -508,8 +517,14 @@ int nvme_set_features(struct nvme_dev *dev, unsigned fid, unsigned dword11,
 	c.features.dword11 = cpu_to_le32(dword11);
 
 	/*
-	 * TODO: add cache flush operation when the size of
-	 * the DMA buffer is known
+	 * TODO: Add a cache clean (aka flush) operation when a DMA buffer is
+	 * involved in the request. The buffer size varies by feature, also
+	 * some features use a different field in the command packet to hold
+	 * the buffer address. Section 5.21.1 (Set Features command) in the
+	 * NVMe specification details the buffer requirements for each
+	 * feature.
+	 * At the moment the only user of this function is not using
+	 * any DMA buffer at all.
 	 */
 
 	return nvme_submit_admin_cmd(dev, &c, result);
-- 
2.17.5