[PATCH v2] cdx: Rename MCDI_LOGGING to CDX_MCDI_LOGGING

[Abhijit Gangurde]

MCDI_LOGGING is too generic considering other MCDI users
SFC_MCDI_LOGGING and SFC_SIENA_MCDI_LOGGING. Rename it to
CDX_MCDI_LOGGING makes it more domain specific.

Also, Move CONFIG_CDX_MCDI_LOGGING to header file
and make logging variable as a configurable sysfs parameter.

Signed-off-by: Abhijit Gangurde <abhijit.gangurde@xxxxxxx>
Suggested-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
---
Changes v1->v2:
- Moved CONFIG_CDX_MCDI_LOGGING flag to header file
- Added sysfs entry to enable/disable mcdi logging

 drivers/cdx/controller/Kconfig          |  2 +-
 drivers/cdx/controller/cdx_controller.c | 21 ++++++++++++
 drivers/cdx/controller/mcdi.c           | 45 ++++++++++++-------------
 drivers/cdx/controller/mcdi.h           |  8 +++--
 4 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/drivers/cdx/controller/Kconfig b/drivers/cdx/controller/Kconfig
index c3e3b9ff8dfe..e7014e9819ea 100644
--- a/drivers/cdx/controller/Kconfig
+++ b/drivers/cdx/controller/Kconfig
@@ -18,7 +18,7 @@ config CDX_CONTROLLER
 
 	  If unsure, say N.
 
-config MCDI_LOGGING
+config CDX_MCDI_LOGGING
 	bool "MCDI Logging for the CDX controller"
 	depends on CDX_CONTROLLER
 	help
diff --git a/drivers/cdx/controller/cdx_controller.c b/drivers/cdx/controller/cdx_controller.c
index dc52f95f8978..e5b1a2c01b87 100644
--- a/drivers/cdx/controller/cdx_controller.c
+++ b/drivers/cdx/controller/cdx_controller.c
@@ -124,6 +124,23 @@ static struct cdx_ops cdx_ops = {
 	.dev_configure	= cdx_configure_device,
 };
 
+static ssize_t mcdi_logging_store(struct device *dev, struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct cdx_controller *cdx = dev_get_drvdata(dev);
+	bool enable;
+
+	if (kstrtobool(buf, &enable) < 0)
+		return -EINVAL;
+
+	if (cdx_enable_mcdi_logging(cdx->priv, enable))
+		return -EINVAL;
+
+	return count;
+}
+
+static DEVICE_ATTR_WO(mcdi_logging);
+
 static int xlnx_cdx_probe(struct platform_device *pdev)
 {
 	struct cdx_controller *cdx;
@@ -154,6 +171,9 @@ static int xlnx_cdx_probe(struct platform_device *pdev)
 	cdx->priv = cdx_mcdi;
 	cdx->ops = &cdx_ops;
 
+	if (device_create_file(&pdev->dev, &dev_attr_mcdi_logging))
+		dev_warn(&pdev->dev, "Failed to create sysfs file\n");
+
 	ret = cdx_setup_rpmsg(pdev);
 	if (ret) {
 		if (ret != -EPROBE_DEFER)
@@ -181,6 +201,7 @@ static int xlnx_cdx_remove(struct platform_device *pdev)
 
 	cdx_destroy_rpmsg(pdev);
 
+	device_remove_file(&pdev->dev, &dev_attr_mcdi_logging);
 	kfree(cdx);
 
 	cdx_mcdi_finish(cdx_mcdi);
diff --git a/drivers/cdx/controller/mcdi.c b/drivers/cdx/controller/mcdi.c
index a211a2ca762e..9afea22c38ee 100644
--- a/drivers/cdx/controller/mcdi.c
+++ b/drivers/cdx/controller/mcdi.c
@@ -31,9 +31,7 @@ struct cdx_mcdi_copy_buffer {
 	struct cdx_dword buffer[DIV_ROUND_UP(MCDI_CTL_SDU_LEN_MAX, 4)];
 };
 
-#ifdef CONFIG_MCDI_LOGGING
 #define LOG_LINE_MAX		(1024 - 32)
-#endif
 
 static void cdx_mcdi_cancel_cmd(struct cdx_mcdi *cdx, struct cdx_mcdi_cmd *cmd);
 static void cdx_mcdi_wait_for_cleanup(struct cdx_mcdi *cdx);
@@ -118,12 +116,12 @@ int cdx_mcdi_init(struct cdx_mcdi *cdx)
 
 	mcdi = cdx_mcdi_if(cdx);
 	mcdi->cdx = cdx;
+	mcdi->logging_enabled = !!CDX_MCDI_LOGGING;
 
-#ifdef CONFIG_MCDI_LOGGING
 	mcdi->logging_buffer = kmalloc(LOG_LINE_MAX, GFP_KERNEL);
 	if (!mcdi->logging_buffer)
 		goto fail2;
-#endif
+
 	mcdi->workqueue = alloc_ordered_workqueue("mcdi_wq", 0);
 	if (!mcdi->workqueue)
 		goto fail3;
@@ -136,10 +134,8 @@ int cdx_mcdi_init(struct cdx_mcdi *cdx)
 
 	return 0;
 fail3:
-#ifdef CONFIG_MCDI_LOGGING
 	kfree(mcdi->logging_buffer);
 fail2:
-#endif
 	kfree(cdx->mcdi);
 	cdx->mcdi = NULL;
 fail:
@@ -155,16 +151,25 @@ void cdx_mcdi_finish(struct cdx_mcdi *cdx)
 		return;
 
 	cdx_mcdi_wait_for_cleanup(cdx);
-
-#ifdef CONFIG_MCDI_LOGGING
 	kfree(mcdi->logging_buffer);
-#endif
 
 	destroy_workqueue(mcdi->workqueue);
 	kfree(cdx->mcdi);
 	cdx->mcdi = NULL;
 }
 
+int cdx_enable_mcdi_logging(struct cdx_mcdi *cdx, bool enable)
+{
+	struct cdx_mcdi_iface *mcdi;
+
+	mcdi = cdx_mcdi_if(cdx);
+	if (!mcdi)
+		return -EINVAL;
+
+	mcdi->logging_enabled = enable;
+	return 0;
+}
+
 static bool cdx_mcdi_flushed(struct cdx_mcdi_iface *mcdi, bool ignore_cleanups)
 {
 	bool flushed;
@@ -246,15 +251,9 @@ static void cdx_mcdi_send_request(struct cdx_mcdi *cdx,
 	size_t hdr_len;
 	bool not_epoch;
 	u32 xflags;
-#ifdef CONFIG_MCDI_LOGGING
-	char *buf;
-#endif
 
 	if (!mcdi)
 		return;
-#ifdef CONFIG_MCDI_LOGGING
-	buf = mcdi->logging_buffer; /* page-sized */
-#endif
 
 	mcdi->prev_seq = cmd->seq;
 	mcdi->seq_held_by[cmd->seq] = cmd;
@@ -281,10 +280,10 @@ static void cdx_mcdi_send_request(struct cdx_mcdi *cdx,
 			     MC_CMD_V2_EXTN_IN_MCDI_MESSAGE_TYPE_PLATFORM);
 	hdr_len = 8;
 
-#ifdef CONFIG_MCDI_LOGGING
-	if (!WARN_ON_ONCE(!buf)) {
+	if (mcdi->logging_enabled) {
 		const struct cdx_dword *frags[] = { hdr, inbuf };
 		const size_t frag_len[] = { hdr_len, round_up(inlen, 4) };
+		char *log = mcdi->logging_buffer;
 		int bytes = 0;
 		int i, j;
 
@@ -300,18 +299,18 @@ static void cdx_mcdi_send_request(struct cdx_mcdi *cdx,
 				 * The string before that is just over 70 bytes.
 				 */
 				if ((bytes + 75) > LOG_LINE_MAX) {
-					pr_info("MCDI RPC REQ:%s \\\n", buf);
+					pr_info("MCDI RPC REQ:%s \\\n", log);
 					bytes = 0;
 				}
-				bytes += snprintf(buf + bytes,
+				bytes += snprintf(log + bytes,
 						  LOG_LINE_MAX - bytes, " %08x",
 						  le32_to_cpu(frag[i].cdx_u32));
 			}
 		}
 
-		pr_info("MCDI RPC REQ:%s\n", buf);
+		pr_info("MCDI RPC REQ:%s\n", log);
 	}
-#endif
+
 	hdr[0].cdx_u32 |= (__force __le32)(cdx_mcdi_payload_csum(hdr, hdr_len, inbuf, inlen) <<
 			 MCDI_HEADER_XFLAGS_LBN);
 	cdx->mcdi_ops->mcdi_request(cdx, hdr, hdr_len, inbuf, inlen);
@@ -700,8 +699,7 @@ static bool cdx_mcdi_complete_cmd(struct cdx_mcdi_iface *mcdi,
 		resp_data_len = 0;
 	}
 
-#ifdef CONFIG_MCDI_LOGGING
-	if (!WARN_ON_ONCE(!mcdi->logging_buffer)) {
+	if (mcdi->logging_enabled) {
 		char *log = mcdi->logging_buffer;
 		int i, bytes = 0;
 		size_t rlen;
@@ -721,7 +719,6 @@ static bool cdx_mcdi_complete_cmd(struct cdx_mcdi_iface *mcdi,
 
 		pr_info("MCDI RPC RESP:%s\n", log);
 	}
-#endif
 
 	if (error && resp_data_len == 0) {
 		/* MC rebooted during command */
diff --git a/drivers/cdx/controller/mcdi.h b/drivers/cdx/controller/mcdi.h
index 0bfbeab04e43..3a22c71e2f31 100644
--- a/drivers/cdx/controller/mcdi.h
+++ b/drivers/cdx/controller/mcdi.h
@@ -22,6 +22,11 @@
 #define CDX_WARN_ON_PARANOID(x) do {} while (0)
 #endif
 
+#ifdef CONFIG_CDX_MCDI_LOGGING
+#define CDX_MCDI_LOGGING	1
+#else
+#define CDX_MCDI_LOGGING	0
+#endif
 /**
  * enum cdx_mcdi_mode - MCDI transaction mode
  * @MCDI_MODE_EVENTS: wait for an mcdi response callback.
@@ -170,10 +175,8 @@ struct cdx_mcdi_iface {
 	enum cdx_mcdi_mode mode;
 	u8 prev_seq;
 	bool new_epoch;
-#ifdef CONFIG_MCDI_LOGGING
 	bool logging_enabled;
 	char *logging_buffer;
-#endif
 };
 
 /**
@@ -193,6 +196,7 @@ static inline struct cdx_mcdi_iface *cdx_mcdi_if(struct cdx_mcdi *cdx)
 
 int cdx_mcdi_init(struct cdx_mcdi *cdx);
 void cdx_mcdi_finish(struct cdx_mcdi *cdx);
+int cdx_enable_mcdi_logging(struct cdx_mcdi *cdx, bool enable);
 
 void cdx_mcdi_process_cmd(struct cdx_mcdi *cdx, struct cdx_dword *outbuf, int len);
 int cdx_mcdi_rpc(struct cdx_mcdi *cdx, unsigned int cmd,
-- 
2.25.1