linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] add support for CDX bus
@ 2023-01-17 13:41 Nipun Gupta
  2023-01-17 13:41 ` [PATCH 01/19] bus/cdx: add the cdx bus driver Nipun Gupta
                   ` (7 more replies)
  0 siblings, 8 replies; 23+ messages in thread
From: Nipun Gupta @ 2023-01-17 13:41 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, gregkh, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git, Nipun Gupta

Introduces AMD CDX bus, which provides a mechanism to
discover/rescan CDX devices. The CDX devices are 
memory mapped on system bus for embedded CPUs.

CDX controller interacts with the firmware to query different
CDX devices present in the Fabric and expose them to the Linux
host on CDX bus.

This patch series:
- Introduces the CDX bus and CDX devices.
- Device tree binding for CDX controller
- Support for CDX bus in arm-smmu-v3 driver
- Add MCDI (Management CPU Driver Interface) as a protocol
  for communication with RPU Firmware
- Support RPMSg channel for Firmware communication

MSI patches for CDX are not added in this series as it's
support is being revisited as per patch series:
https://lore.kernel.org/all/20221111133158.196269823@linutronix.de/
It will be added as separate patches.

RFC changes with stubs were submitted at: 
https://lore.kernel.org/linux-arm-kernel/20221014044049.2557085-1-nipun.gupta@amd.com/

Abhijit Gangurde (1):
  bus/cdx: add rpmsg communication channel for CDX

Nipun Gupta (6):
  bus/cdx: add the cdx bus driver
  iommu/arm-smmu-v3: support ops registration for CDX bus
  dt-bindings: bus: add CDX bus controller device tree bindings
  bus/cdx: add MCDI protocol interface for firmware interaction
  bus/cdx: add cdx controller
  bus/cdx: add device attributes

 Documentation/ABI/testing/sysfs-bus-cdx       |  46 +
 .../bindings/bus/xlnx,cdxbus-controller.yaml  |  68 ++
 MAINTAINERS                                   |   8 +
 drivers/bus/Kconfig                           |   1 +
 drivers/bus/Makefile                          |   2 +
 drivers/bus/cdx/Kconfig                       |  16 +
 drivers/bus/cdx/Makefile                      |   8 +
 drivers/bus/cdx/cdx.c                         | 577 +++++++++++
 drivers/bus/cdx/cdx.h                         |  62 ++
 drivers/bus/cdx/controller/Kconfig            |  30 +
 drivers/bus/cdx/controller/Makefile           |   9 +
 drivers/bus/cdx/controller/bitfield.h         |  88 ++
 drivers/bus/cdx/controller/cdx_controller.c   | 282 ++++++
 drivers/bus/cdx/controller/cdx_controller.h   |  30 +
 drivers/bus/cdx/controller/cdx_rpmsg.c        | 222 +++++
 drivers/bus/cdx/controller/mc_cdx_pcol.h      | 707 ++++++++++++++
 drivers/bus/cdx/controller/mcdi.c             | 918 ++++++++++++++++++
 drivers/bus/cdx/controller/mcdi.h             | 259 +++++
 drivers/bus/cdx/controller/mcdi_functions.c   | 139 +++
 drivers/bus/cdx/controller/mcdi_functions.h   |  61 ++
 drivers/iommu/iommu.c                         |   4 +
 include/linux/cdx/cdx_bus.h                   | 176 ++++
 include/linux/mod_devicetable.h               |  15 +
 scripts/mod/devicetable-offsets.c             |   4 +
 scripts/mod/file2alias.c                      |  12 +
 25 files changed, 3744 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-cdx
 create mode 100644 Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml
 create mode 100644 drivers/bus/cdx/Kconfig
 create mode 100644 drivers/bus/cdx/Makefile
 create mode 100644 drivers/bus/cdx/cdx.c
 create mode 100644 drivers/bus/cdx/cdx.h
 create mode 100644 drivers/bus/cdx/controller/Kconfig
 create mode 100644 drivers/bus/cdx/controller/Makefile
 create mode 100644 drivers/bus/cdx/controller/bitfield.h
 create mode 100644 drivers/bus/cdx/controller/cdx_controller.c
 create mode 100644 drivers/bus/cdx/controller/cdx_controller.h
 create mode 100644 drivers/bus/cdx/controller/cdx_rpmsg.c
 create mode 100644 drivers/bus/cdx/controller/mc_cdx_pcol.h
 create mode 100644 drivers/bus/cdx/controller/mcdi.c
 create mode 100644 drivers/bus/cdx/controller/mcdi.h
 create mode 100644 drivers/bus/cdx/controller/mcdi_functions.c
 create mode 100644 drivers/bus/cdx/controller/mcdi_functions.h
 create mode 100644 include/linux/cdx/cdx_bus.h

-- 
2.17.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 01/19] bus/cdx: add the cdx bus driver
  2023-01-17 13:41 [PATCH 0/7] add support for CDX bus Nipun Gupta
@ 2023-01-17 13:41 ` Nipun Gupta
  2023-01-17 14:07   ` Greg KH
  2023-01-17 17:21   ` Randy Dunlap
  2023-01-17 13:41 ` [PATCH 02/19] iommu/arm-smmu-v3: support ops registration for CDX bus Nipun Gupta
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 23+ messages in thread
From: Nipun Gupta @ 2023-01-17 13:41 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, gregkh, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git, Nipun Gupta

Introduce AMD CDX bus, which provides a mechanism for scanning
and probing CDX devices. These devices are memory mapped on
system bus for Application Processors(APUs).

CDX devices can be changed dynamically in the Fabric and CDX
bus interacts with CDX controller to rescan the bus and
rediscover the devices.

Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
---
 Documentation/ABI/testing/sysfs-bus-cdx |  12 +
 MAINTAINERS                             |   7 +
 drivers/bus/Kconfig                     |   1 +
 drivers/bus/Makefile                    |   2 +
 drivers/bus/cdx/Kconfig                 |  14 +
 drivers/bus/cdx/Makefile                |   8 +
 drivers/bus/cdx/cdx.c                   | 433 ++++++++++++++++++++++++
 drivers/bus/cdx/cdx.h                   |  62 ++++
 include/linux/cdx/cdx_bus.h             | 153 +++++++++
 include/linux/mod_devicetable.h         |  15 +
 scripts/mod/devicetable-offsets.c       |   4 +
 scripts/mod/file2alias.c                |  12 +
 12 files changed, 723 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-cdx
 create mode 100644 drivers/bus/cdx/Kconfig
 create mode 100644 drivers/bus/cdx/Makefile
 create mode 100644 drivers/bus/cdx/cdx.c
 create mode 100644 drivers/bus/cdx/cdx.h
 create mode 100644 include/linux/cdx/cdx_bus.h

diff --git a/Documentation/ABI/testing/sysfs-bus-cdx b/Documentation/ABI/testing/sysfs-bus-cdx
new file mode 100644
index 000000000000..8c2425fdb6d9
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-cdx
@@ -0,0 +1,12 @@
+What:		/sys/bus/cdx/rescan
+Date:		January 2023
+Contact:	nipun.gupta@amd.com
+Description:
+		Writing a non-zero value to this file would cause rescan
+		of the bus and devices on the CDX bus. Any new devices would
+		be scanned and added to the list of linux devices and any
+		devices removed are also deleted from linux.
+
+                For example::
+
+		  # echo 1 > /sys/bus/cdx/rescan
diff --git a/MAINTAINERS b/MAINTAINERS
index 1c83c26e9f68..25c76cf27f21 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -962,6 +962,13 @@ Q:	https://patchwork.kernel.org/project/linux-rdma/list/
 F:	drivers/infiniband/hw/efa/
 F:	include/uapi/rdma/efa-abi.h
 
+AMD CDX BUS DRIVER
+M:	Nipun Gupta <nipun.gupta@amd.com>
+M:	Nikhil Agarwal <nikhil.agarwal@amd.com>
+S:	Maintained
+F:	drivers/bus/cdx/*
+F:	include/linux/cdx/*
+
 AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER
 M:	Tom Lendacky <thomas.lendacky@amd.com>
 M:	John Allen <john.allen@amd.com>
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 7bfe998f3514..b0324efb9a6a 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -251,5 +251,6 @@ config DA8XX_MSTPRI
 
 source "drivers/bus/fsl-mc/Kconfig"
 source "drivers/bus/mhi/Kconfig"
+source "drivers/bus/cdx/Kconfig"
 
 endmenu
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index d90eed189a65..9e4d09789a81 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -20,6 +20,8 @@ obj-$(CONFIG_INTEL_IXP4XX_EB)	+= intel-ixp4xx-eb.o
 obj-$(CONFIG_MIPS_CDMM)		+= mips_cdmm.o
 obj-$(CONFIG_MVEBU_MBUS) 	+= mvebu-mbus.o
 
+obj-$(CONFIG_CDX_BUS)		+= cdx/
+
 # Interconnect bus driver for OMAP SoCs.
 obj-$(CONFIG_OMAP_INTERCONNECT)	+= omap_l3_smx.o omap_l3_noc.o
 
diff --git a/drivers/bus/cdx/Kconfig b/drivers/bus/cdx/Kconfig
new file mode 100644
index 000000000000..54e0623ebcff
--- /dev/null
+++ b/drivers/bus/cdx/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# CDX bus configuration
+#
+# Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+#
+
+config CDX_BUS
+	bool "CDX Bus driver"
+	help
+	  Driver to enable CDX Bus. CDX bus provides a mechanism for
+	  scanning and probing of CDX devices. CDX devices are memory
+	  mapped on system bus for embedded CPUs. CDX bus uses CDX
+	  controller and firmware to scan the CDX devices.
diff --git a/drivers/bus/cdx/Makefile b/drivers/bus/cdx/Makefile
new file mode 100644
index 000000000000..c5feb11fc718
--- /dev/null
+++ b/drivers/bus/cdx/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for CDX
+#
+# Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+#
+
+obj-$(CONFIG_CDX_BUS) += cdx.o
diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
new file mode 100644
index 000000000000..2737470f08d3
--- /dev/null
+++ b/drivers/bus/cdx/cdx.c
@@ -0,0 +1,433 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CDX bus driver.
+ *
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+
+/*
+ * Architecture Overview
+ * =====================
+ * CDX is a Hardware Architecture designed for AMD FPGA devices. It
+ * consists of sophisticated mechanism for interaction between FPGA,
+ * Firmware and the APUs (Application CPUs).
+ *
+ * Firmware resides on RPU (Realtime CPUs) which interacts with
+ * the FPGA program manager and the APUs. The RPU provides memory-mapped
+ * interface (RPU if) which is used to communicate with APUs.
+ *
+ * The diagram below shows an overview of the CDX architecture:
+ *
+ *          +--------------------------------------+
+ *          |    Application CPUs (APU)            |
+ *          |                                      |
+ *          |                    CDX device drivers|
+ *          |     Linux OS                |        |
+ *          |                        CDX bus       |
+ *          |                             |        |
+ *          |                     CDX controller   |
+ *          |                             |        |
+ *          +-----------------------------|--------+
+ *                                        | (discover, config,
+ *                                        |  reset, rescan)
+ *                                        |
+ *          +------------------------| RPU if |----+
+ *          |                             |        |
+ *          |                             V        |
+ *          |          Realtime CPUs (RPU)         |
+ *          |                                      |
+ *          +--------------------------------------+
+ *                                |
+ *          +---------------------|----------------+
+ *          |  FPGA               |                |
+ *          |      +-----------------------+       |
+ *          |      |           |           |       |
+ *          | +-------+    +-------+   +-------+   |
+ *          | | dev 1 |    | dev 2 |   | dev 3 |   |
+ *          | +-------+    +-------+   +-------+   |
+ *          +--------------------------------------+
+ *
+ * The RPU firmware extracts the device information from the loaded FPGA
+ * image and implements a mechanism that allows the APU drivers to
+ * enumerate such devices (device personality and resource details) via
+ * a dedicated communication channel. RPU mediates operations such as
+ * discover, reset and rescan of the FPGA devices for the APU. This is
+ * done using memory mapped interface provided by the RPU to APU.
+ */
+
+#include <linux/init.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/of_device.h>
+#include <linux/slab.h>
+#include <linux/mm.h>
+#include <linux/cdx/cdx_bus.h>
+#include "cdx.h"
+
+/* Default DMA mask for devices on a CDX bus */
+#define CDX_DEFAULT_DMA_MASK	(~0ULL)
+#define MAX_CDX_CONTROLLERS 16
+
+static DECLARE_BITMAP(cdx_controller_id_map, MAX_CDX_CONTROLLERS);
+/* List of CDX controllers registered with the CDX bus */
+static LIST_HEAD(cdx_controllers);
+
+/**
+ * cdx_unregister_device - Unregister a CDX device
+ * @dev: CDX device
+ * @data: This is always passed as NULL, and is not used in this API,
+ *	  but is required here as the bus_for_each_dev() API expects
+ *	  the passed function (cdx_unregister_device) to have this
+ *	  as an argument.
+ *
+ * @return: -errno on failure, 0 on success.
+ */
+static int cdx_unregister_device(struct device *dev,
+				 void *data)
+{
+	struct cdx_device *cdx_dev = to_cdx_device(dev);
+
+	kfree(cdx_dev->driver_override);
+	cdx_dev->driver_override = NULL;
+	/*
+	 * Do not free cdx_dev here as it would be freed in
+	 * cdx_device_release() called from within put_device().
+	 */
+	device_del(&cdx_dev->dev);
+	put_device(&cdx_dev->dev);
+
+	return 0;
+}
+
+static void cdx_unregister_devices(struct bus_type *bus)
+{
+	/* Reset all the devices attached to cdx bus */
+	bus_for_each_dev(bus, NULL, NULL, cdx_unregister_device);
+}
+
+/**
+ * cdx_match_one_device - Tell if a CDX device structure has a matching
+ *			  CDX device id structure
+ * @id: single CDX device id structure to match
+ * @dev: the CDX device structure to match against
+ *
+ * @return: matching cdx_device_id structure or NULL if there is no match.
+ */
+static inline const struct cdx_device_id *
+cdx_match_one_device(const struct cdx_device_id *id,
+		     const struct cdx_device *dev)
+{
+	/* Use vendor ID and device ID for matching */
+	if ((id->vendor == CDX_ANY_ID || id->vendor == dev->vendor) &&
+	    (id->device == CDX_ANY_ID || id->device == dev->device))
+		return id;
+	return NULL;
+}
+
+/**
+ * cdx_match_id - See if a CDX device matches a given cdx_id table
+ * @ids: array of CDX device ID structures to search in
+ * @dev: the CDX device structure to match against.
+ *
+ * Used by a driver to check whether a CDX device is in its list of
+ * supported devices. Returns the matching cdx_device_id structure or
+ * NULL if there is no match.
+ *
+ * @return: matching cdx_device_id structure or NULL if there is no match.
+ */
+static inline const struct cdx_device_id *
+cdx_match_id(const struct cdx_device_id *ids, struct cdx_device *dev)
+{
+	if (ids) {
+		while (ids->vendor || ids->device) {
+			if (cdx_match_one_device(ids, dev))
+				return ids;
+			ids++;
+		}
+	}
+	return NULL;
+}
+
+/**
+ * cdx_bus_match - device to driver matching callback
+ * @dev: the cdx device to match against
+ * @drv: the device driver to search for matching cdx device
+ * structures
+ *
+ * @return: true on success, false otherwise.
+ */
+static int cdx_bus_match(struct device *dev, struct device_driver *drv)
+{
+	struct cdx_device *cdx_dev = to_cdx_device(dev);
+	struct cdx_driver *cdx_drv = to_cdx_driver(drv);
+	const struct cdx_device_id *found_id = NULL;
+	const struct cdx_device_id *ids;
+
+	ids = cdx_drv->match_id_table;
+
+	/* When driver_override is set, only bind to the matching driver */
+	if (cdx_dev->driver_override && strcmp(cdx_dev->driver_override, drv->name))
+		return false;
+
+	found_id = cdx_match_id(ids, cdx_dev);
+	if (!found_id)
+		return false;
+
+	do {
+		/*
+		 * In case override_only was set, enforce driver_override
+		 * matching.
+		 */
+		if (!found_id->override_only)
+			return true;
+		if (cdx_dev->driver_override)
+			return true;
+
+		ids = found_id + 1;
+		found_id = cdx_match_id(ids, cdx_dev);
+	} while (found_id);
+
+	return false;
+}
+
+static int cdx_probe(struct device *dev)
+{
+	struct cdx_driver *cdx_drv = to_cdx_driver(dev->driver);
+	struct cdx_device *cdx_dev = to_cdx_device(dev);
+	int error;
+
+	error = cdx_drv->probe(cdx_dev);
+	if (error < 0) {
+		dev_err_probe(dev, error, "%s failed\n", __func__);
+		return error;
+	}
+
+	return 0;
+}
+
+static void cdx_remove(struct device *dev)
+{
+	struct cdx_driver *cdx_drv = to_cdx_driver(dev->driver);
+	struct cdx_device *cdx_dev = to_cdx_device(dev);
+
+	if (cdx_drv && cdx_drv->remove)
+		cdx_drv->remove(cdx_dev);
+}
+
+static void cdx_shutdown(struct device *dev)
+{
+	struct cdx_driver *cdx_drv = to_cdx_driver(dev->driver);
+	struct cdx_device *cdx_dev = to_cdx_device(dev);
+
+	if (cdx_drv && cdx_drv->shutdown)
+		cdx_drv->shutdown(cdx_dev);
+}
+
+static int cdx_dma_configure(struct device *dev)
+{
+	struct cdx_device *cdx_dev = to_cdx_device(dev);
+	u32 input_id = cdx_dev->req_id;
+	int ret;
+
+	ret = of_dma_configure_id(dev, dev->parent->of_node, 0, &input_id);
+	if (ret && ret != -EPROBE_DEFER) {
+		dev_err(dev, "of_dma_configure_id() failed\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static ssize_t rescan_store(struct bus_type *bus,
+			    const char *buf, size_t count)
+{
+	struct cdx_controller *cdx;
+	unsigned long val = 0;
+
+	if (kstrtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (!val)
+		return -EINVAL;
+
+	/* Unregister all the devices on the bus */
+	cdx_unregister_devices(&cdx_bus_type);
+
+	/* Rescan all the devices */
+	list_for_each_entry(cdx, &cdx_controllers, node) {
+		int ret;
+
+		ret = cdx->ops->scan(cdx);
+		if (ret)
+			dev_err(cdx->dev, "cdx bus scanning failed\n");
+	}
+
+	return count;
+}
+static BUS_ATTR_WO(rescan);
+
+static struct attribute *cdx_bus_attrs[] = {
+	&bus_attr_rescan.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(cdx_bus);
+
+struct bus_type cdx_bus_type = {
+	.name		= "cdx",
+	.match		= cdx_bus_match,
+	.probe		= cdx_probe,
+	.remove		= cdx_remove,
+	.shutdown	= cdx_shutdown,
+	.dma_configure	= cdx_dma_configure,
+	.bus_groups	= cdx_bus_groups,
+};
+EXPORT_SYMBOL_GPL(cdx_bus_type);
+
+int __cdx_driver_register(struct cdx_driver *cdx_driver,
+			  struct module *owner)
+{
+	int error;
+
+	cdx_driver->driver.owner = owner;
+	cdx_driver->driver.bus = &cdx_bus_type;
+
+	error = driver_register(&cdx_driver->driver);
+	if (error < 0) {
+		pr_err("driver_register() failed for %s: %d\n",
+		       cdx_driver->driver.name, error);
+		return error;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__cdx_driver_register);
+
+void cdx_driver_unregister(struct cdx_driver *cdx_driver)
+{
+	driver_unregister(&cdx_driver->driver);
+}
+EXPORT_SYMBOL_GPL(cdx_driver_unregister);
+
+static void cdx_device_release(struct device *dev)
+{
+	struct cdx_device *cdx_dev = to_cdx_device(dev);
+
+	kfree(cdx_dev);
+}
+
+int cdx_device_add(struct cdx_dev_params *dev_params)
+{
+	struct cdx_controller *cdx = dev_params->cdx;
+	struct device *parent = cdx->dev;
+	struct cdx_device *cdx_dev;
+	int ret;
+
+	cdx_dev = kzalloc(sizeof(*cdx_dev), GFP_KERNEL);
+	if (!cdx_dev)
+		return -ENOMEM;
+
+	/* Populate resource */
+	memcpy(cdx_dev->res, dev_params->res, sizeof(struct resource) *
+		dev_params->res_count);
+	cdx_dev->res_count = dev_params->res_count;
+
+	/* Populate CDX dev params */
+	cdx_dev->req_id = dev_params->req_id;
+	cdx_dev->vendor = dev_params->vendor;
+	cdx_dev->device = dev_params->device;
+	cdx_dev->bus_num = dev_params->bus_num;
+	cdx_dev->dev_num = dev_params->dev_num;
+	cdx_dev->cdx = dev_params->cdx;
+	cdx_dev->dma_mask = CDX_DEFAULT_DMA_MASK;
+
+	/* Initiaize generic device */
+	device_initialize(&cdx_dev->dev);
+	cdx_dev->dev.parent = parent;
+	cdx_dev->dev.bus = &cdx_bus_type;
+	cdx_dev->dev.dma_mask = &cdx_dev->dma_mask;
+	cdx_dev->dev.release = cdx_device_release;
+
+	/* Set Name */
+	dev_set_name(&cdx_dev->dev, "cdx-%02x:%02x",
+		     ((cdx->id << CDX_CONTROLLER_ID_SHIFT) | (cdx_dev->bus_num & CDX_BUS_NUM_MASK)),
+		     cdx_dev->dev_num);
+
+	ret = device_add(&cdx_dev->dev);
+	if (ret != 0) {
+		dev_err(&cdx_dev->dev,
+			"cdx device add failed: %d", ret);
+		goto fail;
+	}
+
+	return 0;
+fail:
+	/*
+	 * Do not free cdx_dev here as it would be freed in
+	 * cdx_device_release() called from put_device().
+	 */
+	put_device(&cdx_dev->dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cdx_device_add);
+
+static int get_free_index(void)
+{
+	unsigned long id_map;
+	unsigned long mask;
+	int index = 0;
+
+	mask  = (1UL << MAX_CDX_CONTROLLERS) - 1;
+retry:
+	id_map = cdx_controller_id_map[0];
+	if ((id_map & mask) == mask)
+		return -ENOSPC;
+
+	index = ffz(id_map);
+	if (index >= MAX_CDX_CONTROLLERS)
+		return -ENOSPC;
+
+	if (test_and_set_bit(index, &cdx_controller_id_map[0]))
+		goto retry;
+
+	return index;
+}
+
+int cdx_register_controller(struct cdx_controller *cdx)
+{
+	int ret;
+
+	ret = get_free_index();
+	if (ret < 0) {
+		dev_err(cdx->dev,
+			"No free index available. Maximum controllers already registered\n");
+		cdx->id = (u8)MAX_CDX_CONTROLLERS;
+		return ret;
+	}
+	cdx->id = (u8)ret;
+
+	/* Scan all the devices */
+	cdx->ops->scan(cdx);
+
+	list_add_tail(&cdx->node, &cdx_controllers);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cdx_register_controller);
+
+void cdx_unregister_controller(struct cdx_controller *cdx)
+{
+	if (cdx->id >= MAX_CDX_CONTROLLERS)
+		return;
+
+	device_for_each_child(cdx->dev, NULL, cdx_unregister_device);
+	list_del(&cdx->node);
+	clear_bit(cdx->id, &cdx_controller_id_map[0]);
+}
+EXPORT_SYMBOL_GPL(cdx_unregister_controller);
+
+static int __init cdx_bus_init(void)
+{
+	bitmap_zero(cdx_controller_id_map, MAX_CDX_CONTROLLERS);
+	return bus_register(&cdx_bus_type);
+}
+postcore_initcall(cdx_bus_init);
diff --git a/drivers/bus/cdx/cdx.h b/drivers/bus/cdx/cdx.h
new file mode 100644
index 000000000000..740de8c5212b
--- /dev/null
+++ b/drivers/bus/cdx/cdx.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Header file for the CDX Bus
+ *
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+
+#ifndef _CDX_H_
+#define _CDX_H_
+
+#include <linux/cdx/cdx_bus.h>
+
+/**
+ * struct cdx_dev_params - CDX device parameters
+ * @cdx: CDX controller associated with the device
+ * @parent: Associated CDX controller
+ * @vendor: Vendor ID for CDX device
+ * @device: Device ID for CDX device
+ * @bus_num: Bus number for this CDX device
+ * @dev_num: Device number for this device
+ * @res: array of MMIO region entries
+ * @res_count: number of valid MMIO regions
+ * @req_id: Requestor ID associated with CDX device
+ */
+struct cdx_dev_params {
+	struct cdx_controller *cdx;
+	u16 vendor;
+	u16 device;
+	u8 bus_num;
+	u8 dev_num;
+	struct resource res[MAX_CDX_DEV_RESOURCES];
+	u8 res_count;
+	u32 req_id;
+};
+
+/**
+ * cdx_register_controller - Register a CDX controller and its ports
+ *		on the CDX bus.
+ * @cdx: The CDX controller to register
+ *
+ * @return: -errno on failure, 0 on success.
+ */
+int cdx_register_controller(struct cdx_controller *cdx);
+
+/**
+ * cdx_unregister_controller - Unregister a CDX controller
+ * @cdx: The CDX controller to unregister
+ */
+void cdx_unregister_controller(struct cdx_controller *cdx);
+
+/**
+ * cdx_device_add - Add a CDX device. This function adds a CDX device
+ *		on the CDX bus as per the device parameters provided
+ *		by caller. It also creates and registers an associated
+ *		Linux generic device.
+ * @dev_params: device parameters associated with the device to be created.
+ *
+ * @return: -errno on failure, 0 on success.
+ */
+int cdx_device_add(struct cdx_dev_params *dev_params);
+
+#endif /* _CDX_H_ */
diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
new file mode 100644
index 000000000000..1d28f11da5e7
--- /dev/null
+++ b/include/linux/cdx/cdx_bus.h
@@ -0,0 +1,153 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * CDX bus public interface
+ *
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ *
+ */
+
+#ifndef _CDX_BUS_H_
+#define _CDX_BUS_H_
+
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/mod_devicetable.h>
+
+#define MAX_CDX_DEV_RESOURCES	4
+#define CDX_ANY_ID (0xFFFF)
+#define CDX_CONTROLLER_ID_SHIFT 4
+#define CDX_BUS_NUM_MASK 0xF
+
+/* Forward declaration for CDX controller */
+struct cdx_controller;
+
+typedef int (*cdx_scan_cb)(struct cdx_controller *cdx);
+
+/**
+ * CDX_DEVICE_DRIVER_OVERRIDE - macro used to describe a CDX device with
+ *                              override_only flags.
+ * @vend: the 16 bit CDX Vendor ID
+ * @dev: the 16 bit CDX Device ID
+ * @driver_override: the 32 bit CDX Device override_only
+ *
+ * This macro is used to create a struct cdx_device_id that matches only a
+ * driver_override device.
+ */
+#define CDX_DEVICE_DRIVER_OVERRIDE(vend, dev, driver_override) \
+	.vendor = (vend), .device = (dev), .override_only = (driver_override)
+
+/**
+ * struct cdx_ops - Callbacks supported by CDX controller.
+ * @scan: scan the devices on the controller
+ */
+struct cdx_ops {
+	cdx_scan_cb scan;
+};
+
+/**
+ * struct cdx_controller: CDX controller object
+ * @node: list of CDX controllers
+ * @dev: Linux device associated with the CDX controller.
+ * @priv: private data
+ * @id: Controller ID
+ * @ops: CDX controller ops
+ */
+struct cdx_controller {
+	struct list_head node;
+	struct device *dev;
+	void *priv;
+	u8 id;
+	struct cdx_ops *ops;
+};
+
+/**
+ * struct cdx_device - CDX device object
+ * @dev: Linux driver model device object
+ * @cdx: CDX controller associated with the device
+ * @vendor: Vendor ID for CDX device
+ * @device: Device ID for CDX device
+ * @bus_num: Bus number for this CDX device
+ * @dev_num: Device number for this device
+ * @res: array of MMIO region entries
+ * @res_attr: resource binary attribute
+ * @res_count: number of valid MMIO regions
+ * @dma_mask: Default DMA mask
+ * @flags: CDX device flags
+ * @req_id: Requestor ID associated with CDX device
+ * @driver_override: driver name to force a match; do not set directly,
+ *                   because core frees it; use driver_set_override() to
+ *                   set or clear it.
+ */
+struct cdx_device {
+	struct device dev;
+	struct cdx_controller *cdx;
+	u16 vendor;
+	u16 device;
+	u8 bus_num;
+	u8 dev_num;
+	struct resource res[MAX_CDX_DEV_RESOURCES];
+	u8 res_count;
+	u64 dma_mask;
+	u16 flags;
+	u32 req_id;
+	const char *driver_override;
+};
+
+#define to_cdx_device(_dev) \
+	container_of(_dev, struct cdx_device, dev)
+
+/**
+ * struct cdx_driver - CDX device driver
+ * @driver: Generic device driver
+ * @match_id_table: table of supported device matching Ids
+ * @probe: Function called when a device is added
+ * @remove: Function called when a device is removed
+ * @shutdown: Function called at shutdown time to quiesce the device
+ * @suspend: Function called when a device is stopped
+ * @resume: Function called when a device is resumed
+ * @driver_managed_dma: Device driver doesn't use kernel DMA API for DMA.
+ *		For most device drivers, no need to care about this flag
+ *		as long as all DMAs are handled through the kernel DMA API.
+ *		For some special ones, for example VFIO drivers, they know
+ *		how to manage the DMA themselves and set this flag so that
+ *		the IOMMU layer will allow them to setup and manage their
+ *		own I/O address space.
+ */
+struct cdx_driver {
+	struct device_driver driver;
+	const struct cdx_device_id *match_id_table;
+	int (*probe)(struct cdx_device *dev);
+	int (*remove)(struct cdx_device *dev);
+	void (*shutdown)(struct cdx_device *dev);
+	int (*suspend)(struct cdx_device *dev, pm_message_t state);
+	int (*resume)(struct cdx_device *dev);
+	bool driver_managed_dma;
+};
+
+#define to_cdx_driver(_drv) \
+	container_of(_drv, struct cdx_driver, driver)
+
+/* Macro to avoid include chaining to get THIS_MODULE */
+#define cdx_driver_register(drv) \
+	__cdx_driver_register(drv, THIS_MODULE)
+
+/**
+ * __cdx_driver_register - registers a CDX device driver
+ * @cdx_driver: CDX driver to register
+ * @owner: module owner
+ *
+ * @return: -errno on failure, 0 on success.
+ */
+int __must_check __cdx_driver_register(struct cdx_driver *cdx_driver,
+				       struct module *owner);
+
+/**
+ * cdx_driver_unregister - unregisters a device driver from the
+ * CDX bus.
+ * @cdx_driver: CDX driver to register
+ */
+void cdx_driver_unregister(struct cdx_driver *cdx_driver);
+
+extern struct bus_type cdx_bus_type;
+
+#endif /* _CDX_BUS_H_ */
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 549590e9c644..23445b163c2a 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -911,4 +911,19 @@ struct ishtp_device_id {
 	kernel_ulong_t driver_data;
 };
 
+/**
+ * struct cdx_device_id - CDX device identifier
+ * @vendor: Vendor ID
+ * @device: Device ID
+ * @override_only: Match only when dev->driver_override is this driver.
+ *
+ * Type of entries in the "device Id" table for CDX devices supported by
+ * a CDX device driver.
+ */
+struct cdx_device_id {
+	__u16 vendor;
+	__u16 device;
+	__u32 override_only;
+};
+
 #endif /* LINUX_MOD_DEVICETABLE_H */
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index c0d3bcb99138..62dc988df84d 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -262,5 +262,9 @@ int main(void)
 	DEVID(ishtp_device_id);
 	DEVID_FIELD(ishtp_device_id, guid);
 
+	DEVID(cdx_device_id);
+	DEVID_FIELD(cdx_device_id, vendor);
+	DEVID_FIELD(cdx_device_id, device);
+
 	return 0;
 }
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 91c2e7ba5e52..28da34ba4359 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1452,6 +1452,17 @@ static int do_dfl_entry(const char *filename, void *symval, char *alias)
 	return 1;
 }
 
+/* Looks like: cdx:vNdN */
+static int do_cdx_entry(const char *filename, void *symval,
+			char *alias)
+{
+	DEF_FIELD(symval, cdx_device_id, vendor);
+	DEF_FIELD(symval, cdx_device_id, device);
+
+	sprintf(alias, "cdx:v%08Xd%08Xd", vendor, device);
+	return 1;
+}
+
 /* Does namelen bytes of name exactly match the symbol? */
 static bool sym_is(const char *name, unsigned namelen, const char *symbol)
 {
@@ -1531,6 +1542,7 @@ static const struct devtable devtable[] = {
 	{"ssam", SIZE_ssam_device_id, do_ssam_entry},
 	{"dfl", SIZE_dfl_device_id, do_dfl_entry},
 	{"ishtp", SIZE_ishtp_device_id, do_ishtp_entry},
+	{"cdx", SIZE_cdx_device_id, do_cdx_entry},
 };
 
 /* Create MODULE_ALIAS() statements.
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 02/19] iommu/arm-smmu-v3: support ops registration for CDX bus
  2023-01-17 13:41 [PATCH 0/7] add support for CDX bus Nipun Gupta
  2023-01-17 13:41 ` [PATCH 01/19] bus/cdx: add the cdx bus driver Nipun Gupta
@ 2023-01-17 13:41 ` Nipun Gupta
  2023-01-17 13:41 ` [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree bindings Nipun Gupta
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 23+ messages in thread
From: Nipun Gupta @ 2023-01-17 13:41 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, gregkh, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git, Nipun Gupta

With new CDX bus supported for AMD FPGA devices on ARM
platform, the bus requires registration for the SMMU v3
driver.

Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
---
 drivers/iommu/iommu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 834e6ecf3e51..6a28ef25f337 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -28,6 +28,7 @@
 #include <linux/fsl/mc.h>
 #include <linux/module.h>
 #include <linux/cc_platform.h>
+#include <linux/cdx/cdx_bus.h>
 #include <trace/events/iommu.h>
 #include <linux/sched/mm.h>
 #include <linux/msi.h>
@@ -129,6 +130,9 @@ static struct bus_type * const iommu_buses[] = {
 #ifdef CONFIG_TEGRA_HOST1X_CONTEXT_BUS
 	&host1x_context_device_bus_type,
 #endif
+#ifdef CONFIG_CDX_BUS
+	&cdx_bus_type,
+#endif
 };
 
 /*
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree bindings
  2023-01-17 13:41 [PATCH 0/7] add support for CDX bus Nipun Gupta
  2023-01-17 13:41 ` [PATCH 01/19] bus/cdx: add the cdx bus driver Nipun Gupta
  2023-01-17 13:41 ` [PATCH 02/19] iommu/arm-smmu-v3: support ops registration for CDX bus Nipun Gupta
@ 2023-01-17 13:41 ` Nipun Gupta
  2023-01-17 17:54   ` Krzysztof Kozlowski
  2023-01-17 13:41 ` [PATCH 04/19] bus/cdx: add MCDI protocol interface for firmware interaction Nipun Gupta
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Nipun Gupta @ 2023-01-17 13:41 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, gregkh, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git, Nipun Gupta

Add device tree bindings for CDX bus controller.

Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
---
 .../bindings/bus/xlnx,cdxbus-controller.yaml  | 68 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml

diff --git a/Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml b/Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml
new file mode 100644
index 000000000000..b2f186864021
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/xlnx,cdxbus-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD CDX bus controller
+
+description: |
+  CDX bus controller for AMD devices is implemented to dynamically
+  detect CDX bus and devices on these bus using the firmware.
+  The CDX bus manages multiple FPGA based hardware devices, which
+  can support network, crypto or any other specialized type of
+  devices. These FPGA based devices can be added/modified dynamically
+  on run-time.
+
+  All devices on the CDX bus will have a unique streamid (for IOMMU)
+  and a unique device ID (for MSI) corresponding to a requestor ID
+  (one to one associated with the device). The streamid and deviceid
+  are used to configure SMMU and GIC-ITS respectively.
+
+  iommu-map property is used to define the set of stream ids
+  corresponding to each device and the associated IOMMU.
+
+  The MSI writes are accompanied by sideband data (Device ID).
+  The msi-map property is used to associate the devices with the
+  device ID as well as the associated ITS controller.
+
+  rproc property (xlnx,rproc) is used to identify the remote processor
+  with which APU (Application Processor Unit) interacts to find out
+  the bus and device configuration.
+
+maintainers:
+  - Nipun Gupta <nipun.gupta@amd.com>
+  - Nikhil Agarwal <nikhil.agarwal@amd.com>
+
+properties:
+  compatible:
+    const: xlnx,cdxbus-controller
+
+  iommu-map: true
+
+  msi-map: true
+
+  xlnx,rproc:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to the remoteproc_r5 rproc node using which APU interacts
+      with remote processor.
+
+required:
+  - compatible
+  - iommu-map
+  - msi-map
+  - xlnx,rproc
+
+additionalProperties: false
+
+examples:
+  - |
+    cdxbus-controller {
+        compatible = "xlnx,cdxbus-controller";
+        /* define map for RIDs 250-259 */
+        iommu-map = <250 &smmu 250 10>;
+        /* define msi map for RIDs 250-259 */
+        msi-map = <250 &its 250 10>;
+        xlnx,rproc = <&remoteproc_r5>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 25c76cf27f21..b43242546b17 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -966,6 +966,7 @@ AMD CDX BUS DRIVER
 M:	Nipun Gupta <nipun.gupta@amd.com>
 M:	Nikhil Agarwal <nikhil.agarwal@amd.com>
 S:	Maintained
+F:	Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml
 F:	drivers/bus/cdx/*
 F:	include/linux/cdx/*
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 04/19] bus/cdx: add MCDI protocol interface for firmware interaction
  2023-01-17 13:41 [PATCH 0/7] add support for CDX bus Nipun Gupta
                   ` (2 preceding siblings ...)
  2023-01-17 13:41 ` [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree bindings Nipun Gupta
@ 2023-01-17 13:41 ` Nipun Gupta
  2023-01-17 14:08   ` Greg KH
  2023-01-17 13:41 ` [PATCH 05/19] bus/cdx: add cdx controller Nipun Gupta
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Nipun Gupta @ 2023-01-17 13:41 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, gregkh, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git, Nipun Gupta

The MCDI (Management CPU Driver Interface) is used as a
protocol to communicate with the RPU firmware. It has
pre-defined set of messages for different message exchanges
between APU and RPU.

Signed-off-by: Puneet Gupta <puneet.gupta@amd.com>
Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
---
 drivers/bus/cdx/Kconfig                  |   2 +
 drivers/bus/cdx/Makefile                 |   2 +-
 drivers/bus/cdx/controller/Kconfig       |  20 +
 drivers/bus/cdx/controller/Makefile      |   9 +
 drivers/bus/cdx/controller/bitfield.h    |  88 +++
 drivers/bus/cdx/controller/mc_cdx_pcol.h | 590 +++++++++++++++
 drivers/bus/cdx/controller/mcdi.c        | 918 +++++++++++++++++++++++
 drivers/bus/cdx/controller/mcdi.h        | 250 ++++++
 8 files changed, 1878 insertions(+), 1 deletion(-)
 create mode 100644 drivers/bus/cdx/controller/Kconfig
 create mode 100644 drivers/bus/cdx/controller/Makefile
 create mode 100644 drivers/bus/cdx/controller/bitfield.h
 create mode 100644 drivers/bus/cdx/controller/mc_cdx_pcol.h
 create mode 100644 drivers/bus/cdx/controller/mcdi.c
 create mode 100644 drivers/bus/cdx/controller/mcdi.h

diff --git a/drivers/bus/cdx/Kconfig b/drivers/bus/cdx/Kconfig
index 54e0623ebcff..a7e474d621e0 100644
--- a/drivers/bus/cdx/Kconfig
+++ b/drivers/bus/cdx/Kconfig
@@ -12,3 +12,5 @@ config CDX_BUS
 	  scanning and probing of CDX devices. CDX devices are memory
 	  mapped on system bus for embedded CPUs. CDX bus uses CDX
 	  controller and firmware to scan the CDX devices.
+
+source "drivers/bus/cdx/controller/Kconfig"
diff --git a/drivers/bus/cdx/Makefile b/drivers/bus/cdx/Makefile
index c5feb11fc718..0324e4914f6e 100644
--- a/drivers/bus/cdx/Makefile
+++ b/drivers/bus/cdx/Makefile
@@ -5,4 +5,4 @@
 # Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
 #
 
-obj-$(CONFIG_CDX_BUS) += cdx.o
+obj-$(CONFIG_CDX_BUS) += cdx.o controller/
diff --git a/drivers/bus/cdx/controller/Kconfig b/drivers/bus/cdx/controller/Kconfig
new file mode 100644
index 000000000000..785c71063b2a
--- /dev/null
+++ b/drivers/bus/cdx/controller/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# CDX controller configuration
+#
+# Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+#
+
+if CDX_BUS
+
+config MCDI_LOGGING
+	bool "MCDI Logging for the CDX controller"
+	depends on CDX_CONTROLLER
+	help
+	  Enable MCDI Logging for
+	  the CDX Controller for debug
+	  purpose.
+
+	  If unsure, say N.
+
+endif
diff --git a/drivers/bus/cdx/controller/Makefile b/drivers/bus/cdx/controller/Makefile
new file mode 100644
index 000000000000..0ce200678eda
--- /dev/null
+++ b/drivers/bus/cdx/controller/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Makefile for CDX controller drivers
+#
+# Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+#
+
+obj-$(CONFIG_CDX_CONTROLLER) += cdx-controller.o
+cdx-controller-objs := mcdi.o
diff --git a/drivers/bus/cdx/controller/bitfield.h b/drivers/bus/cdx/controller/bitfield.h
new file mode 100644
index 000000000000..d390e48239d5
--- /dev/null
+++ b/drivers/bus/cdx/controller/bitfield.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright 2005-2006 Fen Systems Ltd.
+ * Copyright 2006-2013 Solarflare Communications Inc.
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+
+#ifndef CDX_BITFIELD_H
+#define CDX_BITFIELD_H
+
+/* Lowest bit numbers and widths */
+#define CDX_DWORD_LBN 0
+#define CDX_DWORD_WIDTH 32
+
+/* Specified attribute (e.g. LBN) of the specified field */
+#define CDX_VAL(field, attribute) field ## _ ## attribute
+/* Low bit number of the specified field */
+#define CDX_LOW_BIT(field) CDX_VAL(field, LBN)
+/* Bit width of the specified field */
+#define CDX_WIDTH(field) CDX_VAL(field, WIDTH)
+/* High bit number of the specified field */
+#define CDX_HIGH_BIT(field) (CDX_LOW_BIT(field) + CDX_WIDTH(field) - 1)
+
+/* A doubleword (i.e. 4 byte) datatype - little-endian in HW */
+struct cdx_dword {
+	__le32 cdx_u32;
+};
+
+/* Value expanders for printk */
+#define CDX_DWORD_VAL(dword)				\
+	((unsigned int)le32_to_cpu((dword).cdx_u32))
+
+/*
+ * Extract bit field portion [low,high) from the 32-bit little-endian
+ * element which contains bits [min,max)
+ */
+#define CDX_DWORD_FIELD(dword, field)					\
+	(FIELD_GET(GENMASK(CDX_HIGH_BIT(field), CDX_LOW_BIT(field)),	\
+		  (dword).cdx_u32))
+
+/*
+ * Creates the portion of the named bit field that lies within the
+ * range [min,max).
+ */
+#define CDX_INSERT_FIELD(field, value)				\
+	(FIELD_PREP(GENMASK(CDX_HIGH_BIT(field),		\
+			    CDX_LOW_BIT(field)), value))
+
+/*
+ * Creates the portion of the named bit fields that lie within the
+ * range [min,max).
+ */
+#define CDX_INSERT_FIELDS(field1, value1,		\
+			  field2, value2,		\
+			  field3, value3,		\
+			  field4, value4,		\
+			  field5, value5,		\
+			  field6, value6,		\
+			  field7, value7)		\
+	(CDX_INSERT_FIELD(field1, (value1)) |		\
+	 CDX_INSERT_FIELD(field2, (value2)) |		\
+	 CDX_INSERT_FIELD(field3, (value3)) |		\
+	 CDX_INSERT_FIELD(field4, (value4)) |		\
+	 CDX_INSERT_FIELD(field5, (value5)) |		\
+	 CDX_INSERT_FIELD(field6, (value6)) |		\
+	 CDX_INSERT_FIELD(field7, (value7)))
+
+#define CDX_POPULATE_DWORD(dword, ...)					\
+	(dword).cdx_u32 = cpu_to_le32(CDX_INSERT_FIELDS(__VA_ARGS__))
+
+/* Populate a dword field with various numbers of arguments */
+#define CDX_POPULATE_DWORD_7 CDX_POPULATE_DWORD
+#define CDX_POPULATE_DWORD_6(dword, ...) \
+	CDX_POPULATE_DWORD_7(dword, CDX_DWORD, 0, __VA_ARGS__)
+#define CDX_POPULATE_DWORD_5(dword, ...) \
+	CDX_POPULATE_DWORD_6(dword, CDX_DWORD, 0, __VA_ARGS__)
+#define CDX_POPULATE_DWORD_4(dword, ...) \
+	CDX_POPULATE_DWORD_5(dword, CDX_DWORD, 0, __VA_ARGS__)
+#define CDX_POPULATE_DWORD_3(dword, ...) \
+	CDX_POPULATE_DWORD_4(dword, CDX_DWORD, 0, __VA_ARGS__)
+#define CDX_POPULATE_DWORD_2(dword, ...) \
+	CDX_POPULATE_DWORD_3(dword, CDX_DWORD, 0, __VA_ARGS__)
+#define CDX_POPULATE_DWORD_1(dword, ...) \
+	CDX_POPULATE_DWORD_2(dword, CDX_DWORD, 0, __VA_ARGS__)
+#define CDX_SET_DWORD(dword) \
+	CDX_POPULATE_DWORD_1(dword, CDX_DWORD, 0xffffffff)
+
+#endif /* CDX_BITFIELD_H */
diff --git a/drivers/bus/cdx/controller/mc_cdx_pcol.h b/drivers/bus/cdx/controller/mc_cdx_pcol.h
new file mode 100644
index 000000000000..902c856b1c5c
--- /dev/null
+++ b/drivers/bus/cdx/controller/mc_cdx_pcol.h
@@ -0,0 +1,590 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Driver for AMD network controllers and boards
+ *
+ * Copyright (C) 2021, Xilinx, Inc.
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+
+#ifndef MC_CDX_PCOL_H
+#define MC_CDX_PCOL_H
+
+/* The current version of the MCDI protocol. */
+#define MCDI_PCOL_VERSION 2
+
+/*
+ * Each MCDI request starts with an MCDI_HEADER, which is a 32bit
+ * structure, filled in by the client.
+ *
+ *       0       7  8     16    20     22  23  24    31
+ *      | CODE | R | LEN | SEQ | Rsvd | E | R | XFLAGS |
+ *               |                      |   |
+ *               |                      |   \--- Response
+ *               |                      \------- Error
+ *               \------------------------------ Resync (always set)
+ *
+ * The client writes its request into MC shared memory, and rings the
+ * doorbell. Each request is completed either by the MC writing
+ * back into shared memory, or by writing out an event.
+ *
+ * All MCDI commands support completion by shared memory response. Each
+ * request may also contain additional data (accounted for by HEADER.LEN),
+ * and some responses may also contain additional data (again, accounted
+ * for by HEADER.LEN).
+ *
+ * Some MCDI commands support completion by event, in which any associated
+ * response data is included in the event.
+ *
+ * The protocol requires one response to be delivered for every request; a
+ * request should not be sent unless the response for the previous request
+ * has been received (either by polling shared memory, or by receiving
+ * an event).
+ */
+
+/** Request/Response structure */
+#define MCDI_HEADER_OFST 0
+#define MCDI_HEADER_CODE_LBN 0
+#define MCDI_HEADER_CODE_WIDTH 7
+#define MCDI_HEADER_RESYNC_LBN 7
+#define MCDI_HEADER_RESYNC_WIDTH 1
+#define MCDI_HEADER_DATALEN_LBN 8
+#define MCDI_HEADER_DATALEN_WIDTH 8
+#define MCDI_HEADER_SEQ_LBN 16
+#define MCDI_HEADER_SEQ_WIDTH 4
+#define MCDI_HEADER_RSVD_LBN 20
+#define MCDI_HEADER_RSVD_WIDTH 1
+#define MCDI_HEADER_NOT_EPOCH_LBN 21
+#define MCDI_HEADER_NOT_EPOCH_WIDTH 1
+#define MCDI_HEADER_ERROR_LBN 22
+#define MCDI_HEADER_ERROR_WIDTH 1
+#define MCDI_HEADER_RESPONSE_LBN 23
+#define MCDI_HEADER_RESPONSE_WIDTH 1
+#define MCDI_HEADER_XFLAGS_LBN 24
+#define MCDI_HEADER_XFLAGS_WIDTH 8
+/* Request response using event */
+#define MCDI_HEADER_XFLAGS_EVREQ 0x01
+/* Request (and signal) early doorbell return */
+#define MCDI_HEADER_XFLAGS_DBRET 0x02
+
+/* Maximum number of payload bytes */
+#define MCDI_CTL_SDU_LEN_MAX_V2 0x400
+
+#define MCDI_CTL_SDU_LEN_MAX MCDI_CTL_SDU_LEN_MAX_V2
+
+/*
+ * The MC can generate events for two reasons:
+ *   - To advance a shared memory request if XFLAGS_EVREQ was set
+ *   - As a notification (link state, i2c event), controlled
+ *     via MC_CMD_LOG_CTRL
+ *
+ * Both events share a common structure:
+ *
+ *  0      32     33      36    44     52     60
+ * | Data | Cont | Level | Src | Code | Rsvd |
+ *           |
+ *           \ There is another event pending in this notification
+ *
+ * If Code==CMDDONE, then the fields are further interpreted as:
+ *
+ *   - LEVEL==INFO    Command succeeded
+ *   - LEVEL==ERR     Command failed
+ *
+ *    0     8         16      24     32
+ *   | Seq | Datalen | Errno | Rsvd |
+ *
+ *   These fields are taken directly out of the standard MCDI header, i.e.,
+ *   LEVEL==ERR, Datalen == 0 => Reboot
+ *
+ * Events can be squirted out of the UART (using LOG_CTRL) without a
+ * MCDI header.  An event can be distinguished from a MCDI response by
+ * examining the first byte which is 0xc0.  This corresponds to the
+ * non-existent MCDI command MC_CMD_DEBUG_LOG.
+ *
+ *      0         7        8
+ *     | command | Resync |     = 0xc0
+ *
+ * Since the event is written in big-endian byte order, this works
+ * providing bits 56-63 of the event are 0xc0.
+ *
+ *      56     60  63
+ *     | Rsvd | Code |    = 0xc0
+ *
+ * Which means for convenience the event code is 0xc for all MC
+ * generated events.
+ */
+
+/*
+ * the errno value may be followed by the (0-based) number of the
+ * first argument that could not be processed.
+ */
+#define MC_CMD_ERR_ARG_OFST 4
+
+/* MC_CMD_ERR enum: MCDI error codes. */
+/* enum: Operation not permitted. */
+#define MC_CMD_ERR_EPERM 0x1
+/* enum: Non-existent command target */
+#define MC_CMD_ERR_ENOENT 0x2
+/* enum: assert() has killed the MC */
+#define MC_CMD_ERR_EINTR 0x4
+/* enum: I/O failure */
+#define MC_CMD_ERR_EIO 0x5
+/* enum: Already exists */
+#define MC_CMD_ERR_EEXIST 0x6
+/* enum: Try again */
+#define MC_CMD_ERR_EAGAIN 0xb
+/* enum: Out of memory */
+#define MC_CMD_ERR_ENOMEM 0xc
+/* enum: Caller does not hold required locks */
+#define MC_CMD_ERR_EACCES 0xd
+/* enum: Resource is currently unavailable (e.g. lock contention) */
+#define MC_CMD_ERR_EBUSY 0x10
+/* enum: No such device */
+#define MC_CMD_ERR_ENODEV 0x13
+/* enum: Invalid argument to target */
+#define MC_CMD_ERR_EINVAL 0x16
+/* enum: No space */
+#define MC_CMD_ERR_ENOSPC 0x1c
+/* enum: Read-only */
+#define MC_CMD_ERR_EROFS 0x1e
+/* enum: Broken pipe */
+#define MC_CMD_ERR_EPIPE 0x20
+/* enum: Out of range */
+#define MC_CMD_ERR_ERANGE 0x22
+/* enum: Non-recursive resource is already acquired */
+#define MC_CMD_ERR_EDEADLK 0x23
+/* enum: Operation not implemented */
+#define MC_CMD_ERR_ENOSYS 0x26
+/* enum: Operation timed out */
+#define MC_CMD_ERR_ETIME 0x3e
+/* enum: Link has been severed */
+#define MC_CMD_ERR_ENOLINK 0x43
+/* enum: Protocol error */
+#define MC_CMD_ERR_EPROTO 0x47
+/* enum: Bad message */
+#define MC_CMD_ERR_EBADMSG 0x4a
+/* enum: Operation not supported */
+#define MC_CMD_ERR_ENOTSUP 0x5f
+/* enum: Address not available */
+#define MC_CMD_ERR_EADDRNOTAVAIL 0x63
+/* enum: Not connected */
+#define MC_CMD_ERR_ENOTCONN 0x6b
+/* enum: Operation already in progress */
+#define MC_CMD_ERR_EALREADY 0x72
+/* enum: Stale handle. The handle references resource that no longer exists */
+#define MC_CMD_ERR_ESTALE 0x74
+/* enum: Resource allocation failed. */
+#define MC_CMD_ERR_ALLOC_FAIL 0x1000
+/* enum: V-adaptor not found. */
+#define MC_CMD_ERR_NO_VADAPTOR 0x1001
+/* enum: EVB port not found. */
+#define MC_CMD_ERR_NO_EVB_PORT 0x1002
+/* enum: V-switch not found. */
+#define MC_CMD_ERR_NO_VSWITCH 0x1003
+/* enum: Too many VLAN tags. */
+#define MC_CMD_ERR_VLAN_LIMIT 0x1004
+/* enum: Bad PCI function number. */
+#define MC_CMD_ERR_BAD_PCI_FUNC 0x1005
+/* enum: Invalid VLAN mode. */
+#define MC_CMD_ERR_BAD_VLAN_MODE 0x1006
+/* enum: Invalid v-switch type. */
+#define MC_CMD_ERR_BAD_VSWITCH_TYPE 0x1007
+/* enum: Invalid v-port type. */
+#define MC_CMD_ERR_BAD_VPORT_TYPE 0x1008
+/* enum: MAC address exists. */
+#define MC_CMD_ERR_MAC_EXIST 0x1009
+/* enum: Slave core not present */
+#define MC_CMD_ERR_SLAVE_NOT_PRESENT 0x100a
+/* enum: The datapath is disabled. */
+#define MC_CMD_ERR_DATAPATH_DISABLED 0x100b
+/* enum: The requesting client is not a function */
+#define MC_CMD_ERR_CLIENT_NOT_FN 0x100c
+/*
+ * enum: The requested operation might require the command to be passed between
+ * MCs, and thetransport doesn't support that. Should only ever been seen over
+ * the UART.
+ */
+#define MC_CMD_ERR_NO_PRIVILEGE 0x1013
+/*
+ * enum: Workaround 26807 could not be turned on/off because some functions
+ * have already installed filters. See the comment at
+ * MC_CMD_WORKAROUND_BUG26807. May also returned for other operations such as
+ * sub-variant switching.
+ */
+#define MC_CMD_ERR_FILTERS_PRESENT 0x1014
+/* enum: The clock whose frequency you've attempted to set doesn't exist */
+#define MC_CMD_ERR_NO_CLOCK 0x1015
+/*
+ * enum: Returned by MC_CMD_TESTASSERT if the action that should have caused an
+ * assertion failed to do so.
+ */
+#define MC_CMD_ERR_UNREACHABLE 0x1016
+/*
+ * enum: This command needs to be processed in the background but there were no
+ * resources to do so. Send it again after a command has completed.
+ */
+#define MC_CMD_ERR_QUEUE_FULL 0x1017
+/*
+ * enum: The operation could not be completed because the PCIe link has gone
+ * away. This error code is never expected to be returned over the TLP
+ * transport.
+ */
+#define MC_CMD_ERR_NO_PCIE 0x1018
+/*
+ * enum: The operation could not be completed because the datapath has gone
+ * away. This is distinct from MC_CMD_ERR_DATAPATH_DISABLED in that the
+ * datapath absence may be temporary
+ */
+#define MC_CMD_ERR_NO_DATAPATH 0x1019
+/* enum: The operation could not complete because some VIs are allocated */
+#define MC_CMD_ERR_VIS_PRESENT 0x101a
+/*
+ * enum: The operation could not complete because some PIO buffers are
+ * allocated
+ */
+#define MC_CMD_ERR_PIOBUFS_PRESENT 0x101b
+
+/***********************************/
+/*
+ * MC_CMD_CDX_BUS_ENUM_BUSES
+ * CDX bus hosts devices (functions) that are implemented using the Composable
+ * DMA subsystem and directly mapped into the memory space of the FGPA PSX
+ * Application Processors (APUs). As such, they only apply to the PSX APU side,
+ * not the host (PCIe). Unlike PCIe, these devices have no native configuration
+ * space or enumeration mechanism, so this message set provides a minimal
+ * interface for discovery and management (bus reset, FLR, BME) of such
+ * devices. This command returns the number of CDX buses present in the system.
+ */
+#define MC_CMD_CDX_BUS_ENUM_BUSES 0x1
+#define MC_CMD_CDX_BUS_ENUM_BUSES_MSGSET 0x1
+#undef MC_CMD_0x1_PRIVILEGE_CTG
+
+#define MC_CMD_0x1_PRIVILEGE_CTG SRIOV_CTG_ADMIN
+
+/* MC_CMD_CDX_BUS_ENUM_BUSES_IN msgrequest */
+#define MC_CMD_CDX_BUS_ENUM_BUSES_IN_LEN 0
+
+/* MC_CMD_CDX_BUS_ENUM_BUSES_OUT msgresponse */
+#define MC_CMD_CDX_BUS_ENUM_BUSES_OUT_LEN 4
+/*
+ * Number of CDX buses present in the system. Buses are numbered 0 to
+ * BUS_COUNT-1
+ */
+#define MC_CMD_CDX_BUS_ENUM_BUSES_OUT_BUS_COUNT_OFST 0
+#define MC_CMD_CDX_BUS_ENUM_BUSES_OUT_BUS_COUNT_LEN 4
+
+/***********************************/
+/*
+ * MC_CMD_CDX_BUS_ENUM_DEVICES
+ * Enumerate CDX bus devices on a given bus
+ */
+#define MC_CMD_CDX_BUS_ENUM_DEVICES 0x2
+#define MC_CMD_CDX_BUS_ENUM_DEVICES_MSGSET 0x2
+#undef MC_CMD_0x2_PRIVILEGE_CTG
+
+#define MC_CMD_0x2_PRIVILEGE_CTG SRIOV_CTG_ADMIN
+
+/* MC_CMD_CDX_BUS_ENUM_DEVICES_IN msgrequest */
+#define MC_CMD_CDX_BUS_ENUM_DEVICES_IN_LEN 4
+/*
+ * Bus number to enumerate, in range 0 to BUS_COUNT-1, as returned by
+ * MC_CMD_CDX_BUS_ENUM_BUSES_OUT
+ */
+#define MC_CMD_CDX_BUS_ENUM_DEVICES_IN_BUS_OFST 0
+#define MC_CMD_CDX_BUS_ENUM_DEVICES_IN_BUS_LEN 4
+
+/* MC_CMD_CDX_BUS_ENUM_DEVICES_OUT msgresponse */
+#define MC_CMD_CDX_BUS_ENUM_DEVICES_OUT_LEN 4
+/*
+ * Number of devices present on the bus. Devices on the bus are numbered 0 to
+ * DEVICE_COUNT-1. Returns EAGAIN if number of devices unknown or if the target
+ * devices are not ready (e.g. undergoing a bus reset)
+ */
+#define MC_CMD_CDX_BUS_ENUM_DEVICES_OUT_DEVICE_COUNT_OFST 0
+#define MC_CMD_CDX_BUS_ENUM_DEVICES_OUT_DEVICE_COUNT_LEN 4
+
+/***********************************/
+/*
+ * MC_CMD_CDX_BUS_GET_DEVICE_CONFIG
+ * Returns device identification and MMIO/MSI resource data for a CDX device.
+ * The expected usage is for the caller to first retrieve the number of devices
+ * on the bus using MC_CMD_BUS_ENUM_DEVICES, then loop through the range (0,
+ * DEVICE_COUNT - 1), retrieving device resource data. May return EAGAIN if the
+ * number of exposed devices or device resources change during enumeration (due
+ * to e.g. a PL reload / bus reset), in which case the caller is expected to
+ * restart the enumeration loop. MMIO addresses are specified in terms of bus
+ * addresses (prior to any potential IOMMU translation). For versal-net, these
+ * are equivalent to APU physical addresses. Implementation note - for this to
+ * work, the implementation needs to keep state (generation count) per client.
+ */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG 0x3
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_MSGSET 0x3
+#undef MC_CMD_0x3_PRIVILEGE_CTG
+
+#define MC_CMD_0x3_PRIVILEGE_CTG SRIOV_CTG_ADMIN
+
+/* MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_IN msgrequest */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_IN_LEN 8
+/* Device bus number, in range 0 to BUS_COUNT-1 */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_IN_BUS_OFST 0
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_IN_BUS_LEN 4
+/* Device number relative to the bus, in range 0 to DEVICE_COUNT-1 for that bus */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_IN_DEVICE_OFST 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_IN_DEVICE_LEN 4
+
+/* MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT msgresponse */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_LEN 88
+/* 16-bit Vendor identifier, compliant with PCI-SIG VendorID assignment. */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_VENDOR_ID_OFST 0
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_VENDOR_ID_LEN 2
+/* 16-bit Device ID assigned by the vendor */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_DEVICE_ID_OFST 2
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_DEVICE_ID_LEN 2
+/*
+ * 16-bit Subsystem Vendor ID, , compliant with PCI-SIG VendorID assignment.
+ * For further device differentiation, as required. 0 if unused.
+ */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_SUBSYS_VENDOR_ID_OFST 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_SUBSYS_VENDOR_ID_LEN 2
+/*
+ * 16-bit Subsystem Device ID assigned by the vendor. For further device
+ * differentiation, as required. 0 if unused.
+ */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_SUBSYS_DEVICE_ID_OFST 6
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_SUBSYS_DEVICE_ID_LEN 2
+/* 24-bit Device Class code, compliant with PCI-SIG Device Class codes */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_DEVICE_CLASS_OFST 8
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_DEVICE_CLASS_LEN 3
+/* 8-bit vendor-assigned revision */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_DEVICE_REVISION_OFST 11
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_DEVICE_REVISION_LEN 1
+/* Reserved (alignment) */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_RESERVED_OFST 12
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_RESERVED_LEN 4
+/* MMIO region 0 base address (bus address), 0 if unused */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE_OFST 16
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE_LEN 8
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE_LO_OFST 16
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE_LO_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE_LO_LBN 128
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE_LO_WIDTH 32
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE_HI_OFST 20
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE_HI_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE_HI_LBN 160
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE_HI_WIDTH 32
+/* MMIO region 0 size, 0 if unused */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE_OFST 24
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE_LEN 8
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE_LO_OFST 24
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE_LO_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE_LO_LBN 192
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE_LO_WIDTH 32
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE_HI_OFST 28
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE_HI_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE_HI_LBN 224
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE_HI_WIDTH 32
+/* MMIO region 1 base address (bus address), 0 if unused */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE_OFST 32
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE_LEN 8
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE_LO_OFST 32
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE_LO_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE_LO_LBN 256
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE_LO_WIDTH 32
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE_HI_OFST 36
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE_HI_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE_HI_LBN 288
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE_HI_WIDTH 32
+/* MMIO region 1 size, 0 if unused */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE_OFST 40
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE_LEN 8
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE_LO_OFST 40
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE_LO_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE_LO_LBN 320
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE_LO_WIDTH 32
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE_HI_OFST 44
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE_HI_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE_HI_LBN 352
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE_HI_WIDTH 32
+/* MMIO region 2 base address (bus address), 0 if unused */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE_OFST 48
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE_LEN 8
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE_LO_OFST 48
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE_LO_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE_LO_LBN 384
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE_LO_WIDTH 32
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE_HI_OFST 52
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE_HI_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE_HI_LBN 416
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE_HI_WIDTH 32
+/* MMIO region 2 size, 0 if unused */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE_OFST 56
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE_LEN 8
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE_LO_OFST 56
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE_LO_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE_LO_LBN 448
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE_LO_WIDTH 32
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE_HI_OFST 60
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE_HI_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE_HI_LBN 480
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE_HI_WIDTH 32
+/* MMIO region 3 base address (bus address), 0 if unused */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE_OFST 64
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE_LEN 8
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE_LO_OFST 64
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE_LO_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE_LO_LBN 512
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE_LO_WIDTH 32
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE_HI_OFST 68
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE_HI_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE_HI_LBN 544
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE_HI_WIDTH 32
+/* MMIO region 3 size, 0 if unused */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE_OFST 72
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE_LEN 8
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE_LO_OFST 72
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE_LO_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE_LO_LBN 576
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE_LO_WIDTH 32
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE_HI_OFST 76
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE_HI_LEN 4
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE_HI_LBN 608
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE_HI_WIDTH 32
+/* MSI vector count */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MSI_COUNT_OFST 80
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_MSI_COUNT_LEN 4
+/* Requester ID used by device (SMMU StreamID, GIC ITS DeviceID) */
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_REQUESTER_ID_OFST 84
+#define MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_REQUESTER_ID_LEN 4
+
+/***********************************/
+/*
+ * MC_CMD_CDX_DEVICE_RESET
+ * After this call completes, device DMA and interrupts are quiesced, devices
+ * logic is reset in a hardware-specific way and DMA bus mastering is disabled.
+ */
+#define MC_CMD_CDX_DEVICE_RESET 0x6
+#define MC_CMD_CDX_DEVICE_RESET_MSGSET 0x6
+#undef MC_CMD_0x6_PRIVILEGE_CTG
+
+#define MC_CMD_0x6_PRIVILEGE_CTG SRIOV_CTG_ADMIN
+
+/* MC_CMD_CDX_DEVICE_RESET_IN msgrequest */
+#define MC_CMD_CDX_DEVICE_RESET_IN_LEN 8
+/* Device bus number, in range 0 to BUS_COUNT-1 */
+#define MC_CMD_CDX_DEVICE_RESET_IN_BUS_OFST 0
+#define MC_CMD_CDX_DEVICE_RESET_IN_BUS_LEN 4
+/* Device number relative to the bus, in range 0 to DEVICE_COUNT-1 for that bus */
+#define MC_CMD_CDX_DEVICE_RESET_IN_DEVICE_OFST 4
+#define MC_CMD_CDX_DEVICE_RESET_IN_DEVICE_LEN 4
+
+/*
+ * MC_CMD_CDX_DEVICE_RESET_OUT msgresponse: The device is quiesced and all
+ * pending device initiated DMA has completed.
+ */
+#define MC_CMD_CDX_DEVICE_RESET_OUT_LEN 0
+
+/***********************************/
+/*
+ * MC_CMD_CDX_DEVICE_CONTROL_SET
+ * If BUS_MASTER is set to disabled, device DMA and interrupts are quiesced.
+ * Pending DMA requests and MSI interrupts are flushed and no further DMA or
+ * interrupts are issued after this command returns. If BUS_MASTER is set to
+ * enabled, device is allowed to initiate DMA. Whether interrupts are enabled
+ * also depends on the value of MSI_ENABLE bit. Note that, in this case, the
+ * device may start DMA before the host receives and processes the MCDI
+ * response. MSI_ENABLE masks or unmasks device interrupts only. Note that for
+ * interrupts to be delivered to the host, both BUS_MASTER and MSI_ENABLE needs
+ * to be set. MMIO_REGIONS_ENABLE enables or disables host accesses to device
+ * MMIO regions. Note that an implementation is allowed to permanently set this
+ * bit to 1, in which case MC_CMD_CDX_DEVICE_CONTROL_GET will always return 1
+ * for this bit, regardless of the value set here.
+ */
+#define MC_CMD_CDX_DEVICE_CONTROL_SET 0x7
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_MSGSET 0x7
+#undef MC_CMD_0x7_PRIVILEGE_CTG
+
+#define MC_CMD_0x7_PRIVILEGE_CTG SRIOV_CTG_ADMIN
+
+/* MC_CMD_CDX_DEVICE_CONTROL_SET_IN msgrequest */
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_LEN 12
+/* Device bus number, in range 0 to BUS_COUNT-1 */
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_BUS_OFST 0
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_BUS_LEN 4
+/* Device number relative to the bus, in range 0 to DEVICE_COUNT-1 for that bus */
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_DEVICE_OFST 4
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_DEVICE_LEN 4
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_FLAGS_OFST 8
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_FLAGS_LEN 4
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_BUS_MASTER_ENABLE_OFST 8
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_BUS_MASTER_ENABLE_LBN 0
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_BUS_MASTER_ENABLE_WIDTH 1
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_MSI_ENABLE_OFST 8
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_MSI_ENABLE_LBN 1
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_MSI_ENABLE_WIDTH 1
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_MMIO_REGIONS_ENABLE_OFST 8
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_MMIO_REGIONS_ENABLE_LBN 2
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_IN_MMIO_REGIONS_ENABLE_WIDTH 1
+
+/* MC_CMD_CDX_DEVICE_CONTROL_SET_OUT msgresponse */
+#define MC_CMD_CDX_DEVICE_CONTROL_SET_OUT_LEN 0
+
+/***********************************/
+/*
+ * MC_CMD_CDX_DEVICE_CONTROL_GET
+ * Returns device DMA, interrupt and MMIO region access control bits. See
+ * MC_CMD_CDX_DEVICE_CONTROL_SET for definition of the available control bits.
+ */
+#define MC_CMD_CDX_DEVICE_CONTROL_GET 0x8
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_MSGSET 0x8
+#undef MC_CMD_0x8_PRIVILEGE_CTG
+
+#define MC_CMD_0x8_PRIVILEGE_CTG SRIOV_CTG_ADMIN
+
+/* MC_CMD_CDX_DEVICE_CONTROL_GET_IN msgrequest */
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_IN_LEN 8
+/* Device bus number, in range 0 to BUS_COUNT-1 */
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_IN_BUS_OFST 0
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_IN_BUS_LEN 4
+/* Device number relative to the bus, in range 0 to DEVICE_COUNT-1 for that bus */
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_IN_DEVICE_OFST 4
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_IN_DEVICE_LEN 4
+
+/* MC_CMD_CDX_DEVICE_CONTROL_GET_OUT msgresponse */
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_LEN 4
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_FLAGS_OFST 0
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_FLAGS_LEN 4
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_BUS_MASTER_ENABLE_OFST 0
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_BUS_MASTER_ENABLE_LBN 0
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_BUS_MASTER_ENABLE_WIDTH 1
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_MSI_ENABLE_OFST 0
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_MSI_ENABLE_LBN 1
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_MSI_ENABLE_WIDTH 1
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_MMIO_REGIONS_ENABLE_OFST 0
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_MMIO_REGIONS_ENABLE_LBN 2
+#define MC_CMD_CDX_DEVICE_CONTROL_GET_OUT_MMIO_REGIONS_ENABLE_WIDTH 1
+
+/***********************************/
+/* MC_CMD_V2_EXTN - Encapsulation for a v2 extended command */
+#define MC_CMD_V2_EXTN 0x7f
+
+/* MC_CMD_V2_EXTN_IN msgrequest */
+#define MC_CMD_V2_EXTN_IN_LEN 4
+/* the extended command number */
+#define MC_CMD_V2_EXTN_IN_EXTENDED_CMD_LBN 0
+#define MC_CMD_V2_EXTN_IN_EXTENDED_CMD_WIDTH 15
+#define MC_CMD_V2_EXTN_IN_UNUSED_LBN 15
+#define MC_CMD_V2_EXTN_IN_UNUSED_WIDTH 1
+/* the actual length of the encapsulated command */
+#define MC_CMD_V2_EXTN_IN_ACTUAL_LEN_LBN 16
+#define MC_CMD_V2_EXTN_IN_ACTUAL_LEN_WIDTH 10
+#define MC_CMD_V2_EXTN_IN_UNUSED2_LBN 26
+#define MC_CMD_V2_EXTN_IN_UNUSED2_WIDTH 2
+/* Type of command/response */
+#define MC_CMD_V2_EXTN_IN_MESSAGE_TYPE_LBN 28
+#define MC_CMD_V2_EXTN_IN_MESSAGE_TYPE_WIDTH 4
+/*
+ * enum: MCDI command directed to versal-net. MCDI responses of this type
+ * are not defined.
+ */
+#define MC_CMD_V2_EXTN_IN_MCDI_MESSAGE_TYPE_PLATFORM 0x2
+
+#endif /* MC_CDX_PCOL_H */
diff --git a/drivers/bus/cdx/controller/mcdi.c b/drivers/bus/cdx/controller/mcdi.c
new file mode 100644
index 000000000000..49020e79dd1c
--- /dev/null
+++ b/drivers/bus/cdx/controller/mcdi.c
@@ -0,0 +1,918 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Management-Controller-to-Driver Interface
+ *
+ * Copyright 2008-2013 Solarflare Communications Inc.
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/spinlock.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
+#include <linux/timer.h>
+#include <linux/list.h>
+#include <linux/pci.h>
+#include <linux/device.h>
+#include <linux/version.h>
+#include <linux/rwsem.h>
+#include <linux/vmalloc.h>
+#include <net/netevent.h>
+#include <linux/log2.h>
+#include <linux/net_tstamp.h>
+#include <linux/wait.h>
+
+#include "bitfield.h"
+#include "mcdi.h"
+
+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);
+static int cdx_mcdi_rpc_async_internal(struct cdx_mcdi *cdx,
+				       struct cdx_mcdi_cmd *cmd,
+				       unsigned int *handle);
+static void cdx_mcdi_start_or_queue(struct cdx_mcdi_iface *mcdi,
+				    bool allow_retry);
+static void cdx_mcdi_cmd_start_or_queue(struct cdx_mcdi_iface *mcdi,
+					struct cdx_mcdi_cmd *cmd);
+static void cdx_mcdi_cmd_start_or_queue_ext(struct cdx_mcdi_iface *mcdi,
+					    struct cdx_mcdi_cmd *cmd);
+static bool cdx_mcdi_complete_cmd(struct cdx_mcdi_iface *mcdi,
+				  struct cdx_mcdi_cmd *cmd,
+				  struct cdx_mcdi_copy_buffer *copybuf,
+				  struct list_head *cleanup_list);
+static void cdx_mcdi_timeout_cmd(struct cdx_mcdi_iface *mcdi,
+				 struct cdx_mcdi_cmd *cmd,
+				 struct list_head *cleanup_list);
+static void cdx_mcdi_cmd_work(struct work_struct *context);
+static void cdx_mcdi_mode_fail(struct cdx_mcdi *cdx, struct list_head *cleanup_list);
+static void _cdx_mcdi_display_error(struct cdx_mcdi *cdx, unsigned int cmd,
+				    size_t inlen, int raw, int arg, int rc);
+
+static bool cdx_cmd_cancelled(struct cdx_mcdi_cmd *cmd)
+{
+	return cmd->state == MCDI_STATE_RUNNING_CANCELLED;
+}
+
+static void cdx_mcdi_cmd_release(struct kref *ref)
+{
+	kfree(container_of(ref, struct cdx_mcdi_cmd, ref));
+}
+
+static unsigned int cdx_mcdi_cmd_handle(struct cdx_mcdi_cmd *cmd)
+{
+	return cmd->handle;
+}
+
+static void _cdx_mcdi_remove_cmd(struct cdx_mcdi_iface *mcdi,
+				 struct cdx_mcdi_cmd *cmd,
+				 struct list_head *cleanup_list)
+{
+	/* if cancelled, the completers have already been called */
+	if (cdx_cmd_cancelled(cmd))
+		return;
+
+	if (cmd->completer) {
+		list_add_tail(&cmd->cleanup_list, cleanup_list);
+		++mcdi->outstanding_cleanups;
+		kref_get(&cmd->ref);
+	}
+}
+
+static void cdx_mcdi_remove_cmd(struct cdx_mcdi_iface *mcdi,
+				struct cdx_mcdi_cmd *cmd,
+				struct list_head *cleanup_list)
+{
+	list_del(&cmd->list);
+	_cdx_mcdi_remove_cmd(mcdi, cmd, cleanup_list);
+	cmd->state = MCDI_STATE_FINISHED;
+	kref_put(&cmd->ref, cdx_mcdi_cmd_release);
+	if (list_empty(&mcdi->cmd_list))
+		wake_up(&mcdi->cmd_complete_wq);
+}
+
+static unsigned long cdx_mcdi_rpc_timeout(struct cdx_mcdi *cdx, unsigned int cmd)
+{
+	if (!cdx->mcdi_ops->mcdi_rpc_timeout)
+		return MCDI_RPC_TIMEOUT;
+	else
+		return cdx->mcdi_ops->mcdi_rpc_timeout(cdx, cmd);
+}
+
+int cdx_mcdi_init(struct cdx_mcdi *cdx)
+{
+	struct cdx_mcdi_iface *mcdi;
+	int rc = -ENOMEM;
+
+	cdx->mcdi = kzalloc(sizeof(*cdx->mcdi), GFP_KERNEL);
+	if (!cdx->mcdi)
+		goto fail;
+
+	mcdi = cdx_mcdi_if(cdx);
+	mcdi->cdx = cdx;
+
+#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;
+	mutex_init(&mcdi->iface_lock);
+	mcdi->mode = MCDI_MODE_EVENTS;
+	INIT_LIST_HEAD(&mcdi->cmd_list);
+	init_waitqueue_head(&mcdi->cmd_complete_wq);
+
+	mcdi->new_epoch = true;
+
+	return 0;
+fail3:
+#ifdef CONFIG_MCDI_LOGGING
+	kfree(mcdi->logging_buffer);
+fail2:
+#endif
+	kfree(cdx->mcdi);
+	cdx->mcdi = NULL;
+fail:
+	return rc;
+}
+
+void cdx_mcdi_finish(struct cdx_mcdi *cdx)
+{
+	struct cdx_mcdi_iface *iface;
+
+	if (!cdx->mcdi)
+		return;
+
+	cdx_mcdi_wait_for_cleanup(cdx);
+
+	iface = cdx_mcdi_if(cdx);
+#ifdef CONFIG_MCDI_LOGGING
+	kfree(iface->logging_buffer);
+#endif
+
+	destroy_workqueue(iface->workqueue);
+	kfree(cdx->mcdi);
+	cdx->mcdi = NULL;
+}
+
+static bool cdx_mcdi_flushed(struct cdx_mcdi_iface *mcdi, bool ignore_cleanups)
+{
+	bool flushed;
+
+	mutex_lock(&mcdi->iface_lock);
+	flushed = list_empty(&mcdi->cmd_list) &&
+		  (ignore_cleanups || !mcdi->outstanding_cleanups);
+	mutex_unlock(&mcdi->iface_lock);
+	return flushed;
+}
+
+/* Wait for outstanding MCDI commands to complete. */
+static void cdx_mcdi_wait_for_cleanup(struct cdx_mcdi *cdx)
+{
+	struct cdx_mcdi_iface *mcdi = cdx_mcdi_if(cdx);
+
+	if (!mcdi)
+		return;
+
+	wait_event(mcdi->cmd_complete_wq,
+		   cdx_mcdi_flushed(mcdi, false));
+}
+
+int cdx_mcdi_wait_for_quiescence(struct cdx_mcdi *cdx,
+				 unsigned int timeout_jiffies)
+{
+	struct cdx_mcdi_iface *mcdi = cdx_mcdi_if(cdx);
+	DEFINE_WAIT_FUNC(wait, woken_wake_function);
+	int rc;
+
+	if (!mcdi)
+		return -EINVAL;
+
+	flush_workqueue(mcdi->workqueue);
+
+	add_wait_queue(&mcdi->cmd_complete_wq, &wait);
+
+	while (!cdx_mcdi_flushed(mcdi, true)) {
+		rc = wait_woken(&wait, TASK_IDLE, timeout_jiffies);
+		if (rc != 0)
+			continue;
+		break;
+	}
+
+	remove_wait_queue(&mcdi->cmd_complete_wq, &wait);
+
+	if (rc > 0)
+		rc = 0;
+	else if (rc == 0)
+		rc = -ETIMEDOUT;
+
+	return rc;
+}
+
+static u8 cdx_mcdi_payload_csum(const struct cdx_dword *hdr, size_t hdr_len,
+				const struct cdx_dword *sdu, size_t sdu_len)
+{
+	u8 *p = (u8 *)hdr;
+	u8 csum = 0;
+	int i;
+
+	for (i = 0; i < hdr_len; i++)
+		csum += p[i];
+
+	p = (u8 *)sdu;
+	for (i = 0; i < sdu_len; i++)
+		csum += p[i];
+
+	return ~csum & 0xff;
+}
+
+static void cdx_mcdi_send_request(struct cdx_mcdi *cdx,
+				  struct cdx_mcdi_cmd *cmd)
+{
+	struct cdx_mcdi_iface *mcdi = cdx_mcdi_if(cdx);
+	const struct cdx_dword *inbuf = cmd->inbuf;
+	size_t inlen = cmd->inlen;
+	struct cdx_dword hdr[2];
+	size_t hdr_len;
+	u32 xflags;
+#ifdef CONFIG_MCDI_LOGGING
+	char *buf;
+#endif
+
+	if (!mcdi) {
+		pr_err("Caught null pointer for mcdi\n");
+		return;
+	}
+#ifdef CONFIG_MCDI_LOGGING
+	buf = mcdi->logging_buffer; /* page-sized */
+#endif
+
+	mcdi->prev_seq = cmd->seq;
+	mcdi->seq_held_by[cmd->seq] = cmd;
+	mcdi->db_held_by = cmd;
+	cmd->started = jiffies;
+
+	xflags = 0;
+
+	/* MCDI v2 */
+	WARN_ON(inlen > MCDI_CTL_SDU_LEN_MAX_V2);
+	CDX_POPULATE_DWORD_7(hdr[0],
+			     MCDI_HEADER_RESPONSE, 0,
+			     MCDI_HEADER_RESYNC, 1,
+			     MCDI_HEADER_CODE, MC_CMD_V2_EXTN,
+			     MCDI_HEADER_DATALEN, 0,
+			     MCDI_HEADER_SEQ, cmd->seq,
+			     MCDI_HEADER_XFLAGS, xflags,
+			     MCDI_HEADER_NOT_EPOCH, !mcdi->new_epoch);
+	CDX_POPULATE_DWORD_3(hdr[1],
+			     MC_CMD_V2_EXTN_IN_EXTENDED_CMD, cmd->cmd,
+			     MC_CMD_V2_EXTN_IN_ACTUAL_LEN, inlen,
+			     MC_CMD_V2_EXTN_IN_MESSAGE_TYPE,
+			     MC_CMD_V2_EXTN_IN_MCDI_MESSAGE_TYPE_PLATFORM);
+	hdr_len = 8;
+
+#ifdef CONFIG_MCDI_LOGGING
+	if (!WARN_ON_ONCE(!buf)) {
+		const struct cdx_dword *frags[] = { hdr, inbuf };
+		const size_t frag_len[] = { hdr_len, round_up(inlen, 4) };
+		int bytes = 0;
+		int i, j;
+
+		for (j = 0; j < ARRAY_SIZE(frags); j++) {
+			const struct cdx_dword *frag;
+
+			frag = frags[j];
+			for (i = 0;
+			     i < frag_len[j] / 4;
+			     i++) {
+				/*
+				 * Do not exceed the internal printk limit.
+				 * The string before that is just over 70 bytes.
+				 */
+				if ((bytes + 75) > LOG_LINE_MAX) {
+					pr_info("MCDI RPC REQ:%s \\\n", buf);
+					bytes = 0;
+				}
+				bytes += snprintf(buf + bytes,
+						  LOG_LINE_MAX - bytes, " %08x",
+						  le32_to_cpu(frag[i].cdx_u32));
+			}
+		}
+
+		pr_info("MCDI RPC REQ:%s\n", buf);
+	}
+#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, cmd->bufid, hdr, hdr_len, inbuf, inlen);
+
+	mcdi->new_epoch = false;
+}
+
+static int cdx_mcdi_errno(struct cdx_mcdi *cdx, unsigned int mcdi_err)
+{
+	switch (mcdi_err) {
+	case 0:
+	case MC_CMD_ERR_QUEUE_FULL:
+		return mcdi_err;
+	case MC_CMD_ERR_EPERM:
+		return -EPERM;
+	case MC_CMD_ERR_ENOENT:
+		return -ENOENT;
+	case MC_CMD_ERR_EINTR:
+		return -EINTR;
+	case MC_CMD_ERR_EAGAIN:
+		return -EAGAIN;
+	case MC_CMD_ERR_EACCES:
+		return -EACCES;
+	case MC_CMD_ERR_EBUSY:
+		return -EBUSY;
+	case MC_CMD_ERR_EINVAL:
+		return -EINVAL;
+	case MC_CMD_ERR_ERANGE:
+		return -ERANGE;
+	case MC_CMD_ERR_EDEADLK:
+		return -EDEADLK;
+	case MC_CMD_ERR_ENOSYS:
+		return -EOPNOTSUPP;
+	case MC_CMD_ERR_ETIME:
+		return -ETIME;
+	case MC_CMD_ERR_EALREADY:
+		return -EALREADY;
+	case MC_CMD_ERR_ENOSPC:
+		return -ENOSPC;
+	case MC_CMD_ERR_ENOMEM:
+		return -ENOMEM;
+	case MC_CMD_ERR_ENOTSUP:
+		return -EOPNOTSUPP;
+	case MC_CMD_ERR_ALLOC_FAIL:
+		return -ENOBUFS;
+	case MC_CMD_ERR_MAC_EXIST:
+		return -EADDRINUSE;
+	case MC_CMD_ERR_NO_EVB_PORT:
+		return -EAGAIN;
+	default:
+		return -EPROTO;
+	}
+}
+
+static void cdx_mcdi_process_cleanup_list(struct cdx_mcdi *cdx,
+					  struct list_head *cleanup_list)
+{
+	struct cdx_mcdi_iface *mcdi = cdx_mcdi_if(cdx);
+	unsigned int cleanups = 0;
+
+	while (!list_empty(cleanup_list)) {
+		struct cdx_mcdi_cmd *cmd =
+			list_first_entry(cleanup_list,
+					 struct cdx_mcdi_cmd, cleanup_list);
+		cmd->completer(cdx, cmd->cookie, cmd->rc,
+			       cmd->outbuf, cmd->outlen);
+		list_del(&cmd->cleanup_list);
+		kref_put(&cmd->ref, cdx_mcdi_cmd_release);
+		++cleanups;
+	}
+
+	if (cleanups) {
+		bool all_done;
+
+		mutex_lock(&mcdi->iface_lock);
+		CDX_WARN_ON_PARANOID(cleanups > mcdi->outstanding_cleanups);
+		all_done = (mcdi->outstanding_cleanups -= cleanups) == 0;
+		mutex_unlock(&mcdi->iface_lock);
+		if (all_done)
+			wake_up(&mcdi->cmd_complete_wq);
+	}
+}
+
+static void _cdx_mcdi_cancel_cmd(struct cdx_mcdi_iface *mcdi,
+				 unsigned int handle,
+				 struct list_head *cleanup_list)
+{
+	struct cdx_mcdi_cmd *cmd;
+
+	list_for_each_entry(cmd, &mcdi->cmd_list, list)
+		if (cdx_mcdi_cmd_handle(cmd) == handle) {
+			switch (cmd->state) {
+			case MCDI_STATE_QUEUED:
+			case MCDI_STATE_RETRY:
+				pr_debug("command %#x inlen %zu cancelled in queue\n",
+					 cmd->cmd, cmd->inlen);
+				/* if not yet running, properly cancel it */
+				cmd->rc = -EPIPE;
+				cdx_mcdi_remove_cmd(mcdi, cmd, cleanup_list);
+				break;
+			case MCDI_STATE_RUNNING:
+			case MCDI_STATE_RUNNING_CANCELLED:
+			case MCDI_STATE_FINISHED:
+			default:
+				/* invalid state? */
+				WARN_ON(1);
+			}
+			break;
+		}
+}
+
+static void cdx_mcdi_cancel_cmd(struct cdx_mcdi *cdx, struct cdx_mcdi_cmd *cmd)
+{
+	struct cdx_mcdi_iface *mcdi = cdx_mcdi_if(cdx);
+	LIST_HEAD(cleanup_list);
+
+	if (!mcdi)
+		return;
+
+	mutex_lock(&mcdi->iface_lock);
+	cdx_mcdi_timeout_cmd(mcdi, cmd, &cleanup_list);
+	mutex_unlock(&mcdi->iface_lock);
+	cdx_mcdi_process_cleanup_list(cdx, &cleanup_list);
+}
+
+struct cdx_mcdi_blocking_data {
+	struct kref ref;
+	bool done;
+	wait_queue_head_t wq;
+	int rc;
+	struct cdx_dword *outbuf;
+	size_t outlen;
+	size_t outlen_actual;
+};
+
+static void cdx_mcdi_blocking_data_release(struct kref *ref)
+{
+	kfree(container_of(ref, struct cdx_mcdi_blocking_data, ref));
+}
+
+static void cdx_mcdi_rpc_completer(struct cdx_mcdi *cdx, unsigned long cookie,
+				   int rc, struct cdx_dword *outbuf,
+				   size_t outlen_actual)
+{
+	struct cdx_mcdi_blocking_data *wait_data =
+		(struct cdx_mcdi_blocking_data *)cookie;
+
+	wait_data->rc = rc;
+	memcpy(wait_data->outbuf, outbuf,
+	       min(outlen_actual, wait_data->outlen));
+	wait_data->outlen_actual = outlen_actual;
+	/* memory barrier */
+	smp_wmb();
+	wait_data->done = true;
+	wake_up(&wait_data->wq);
+	kref_put(&wait_data->ref, cdx_mcdi_blocking_data_release);
+}
+
+static int cdx_mcdi_rpc_sync(struct cdx_mcdi *cdx, unsigned int cmd,
+			     const struct cdx_dword *inbuf, size_t inlen,
+			     struct cdx_dword *outbuf, size_t outlen,
+			     size_t *outlen_actual, bool quiet)
+{
+	struct cdx_mcdi_blocking_data *wait_data;
+	struct cdx_mcdi_cmd *cmd_item;
+	unsigned int handle;
+	int rc;
+
+	if (outlen_actual)
+		*outlen_actual = 0;
+
+	wait_data = kmalloc(sizeof(*wait_data), GFP_KERNEL);
+	if (!wait_data)
+		return -ENOMEM;
+
+	cmd_item = kmalloc(sizeof(*cmd_item), GFP_KERNEL);
+	if (!cmd_item) {
+		kfree(wait_data);
+		return -ENOMEM;
+	}
+
+	kref_init(&wait_data->ref);
+	wait_data->done = false;
+	init_waitqueue_head(&wait_data->wq);
+	wait_data->outbuf = outbuf;
+	wait_data->outlen = outlen;
+
+	kref_init(&cmd_item->ref);
+	cmd_item->quiet = quiet;
+	cmd_item->cookie = (unsigned long)wait_data;
+	cmd_item->completer = &cdx_mcdi_rpc_completer;
+	cmd_item->cmd = cmd;
+	cmd_item->inlen = inlen;
+	cmd_item->inbuf = inbuf;
+
+	/* Claim an extra reference for the completer to put. */
+	kref_get(&wait_data->ref);
+	rc = cdx_mcdi_rpc_async_internal(cdx, cmd_item, &handle);
+	if (rc) {
+		kref_put(&wait_data->ref, cdx_mcdi_blocking_data_release);
+		goto out;
+	}
+
+	if (!wait_event_timeout(wait_data->wq, wait_data->done,
+				cdx_mcdi_rpc_timeout(cdx, cmd)) &&
+	    !wait_data->done) {
+		pr_err("MC command 0x%x inlen %zu timed out (sync)\n",
+		       cmd, inlen);
+
+		cdx_mcdi_cancel_cmd(cdx, cmd_item);
+
+		wait_data->rc = -ETIMEDOUT;
+		wait_data->outlen_actual = 0;
+	}
+
+	if (outlen_actual)
+		*outlen_actual = wait_data->outlen_actual;
+	rc = wait_data->rc;
+
+out:
+	kref_put(&wait_data->ref, cdx_mcdi_blocking_data_release);
+
+	return rc;
+}
+
+static bool cdx_mcdi_get_seq(struct cdx_mcdi_iface *mcdi, unsigned char *seq)
+{
+	*seq = mcdi->prev_seq;
+	do {
+		*seq = (*seq + 1) % ARRAY_SIZE(mcdi->seq_held_by);
+	} while (mcdi->seq_held_by[*seq] && *seq != mcdi->prev_seq);
+	return !mcdi->seq_held_by[*seq];
+}
+
+static int cdx_mcdi_rpc_async_internal(struct cdx_mcdi *cdx,
+				       struct cdx_mcdi_cmd *cmd,
+				       unsigned int *handle)
+{
+	struct cdx_mcdi_iface *mcdi = cdx_mcdi_if(cdx);
+	LIST_HEAD(cleanup_list);
+
+	if (!mcdi) {
+		kref_put(&cmd->ref, cdx_mcdi_cmd_release);
+		return -ENETDOWN;
+	}
+
+	if (mcdi->mode == MCDI_MODE_FAIL) {
+		kref_put(&cmd->ref, cdx_mcdi_cmd_release);
+		return -ENETDOWN;
+	}
+
+	cmd->mcdi = mcdi;
+	INIT_WORK(&cmd->work, cdx_mcdi_cmd_work);
+	INIT_LIST_HEAD(&cmd->list);
+	INIT_LIST_HEAD(&cmd->cleanup_list);
+	cmd->rc = 0;
+	cmd->outbuf = NULL;
+	cmd->outlen = 0;
+
+	queue_work(mcdi->workqueue, &cmd->work);
+	return 0;
+}
+
+static void cdx_mcdi_cmd_start_or_queue_ext(struct cdx_mcdi_iface *mcdi,
+					    struct cdx_mcdi_cmd *cmd)
+{
+	struct cdx_mcdi *cdx = mcdi->cdx;
+	u8 seq, bufid;
+
+	if (!mcdi->db_held_by &&
+	    cdx_mcdi_get_seq(mcdi, &seq) &&
+	    cdx->mcdi_ops->mcdi_get_buf(cdx, &bufid)) {
+		cmd->seq = seq;
+		cmd->bufid = bufid;
+		cmd->reboot_seen = false;
+		cdx_mcdi_send_request(cdx, cmd);
+		cmd->state = MCDI_STATE_RUNNING;
+	} else {
+		cmd->state = MCDI_STATE_QUEUED;
+	}
+}
+
+static void cdx_mcdi_cmd_start_or_queue(struct cdx_mcdi_iface *mcdi,
+					struct cdx_mcdi_cmd *cmd)
+{
+	cdx_mcdi_cmd_start_or_queue_ext(mcdi, cmd);
+}
+
+/* try to advance other commands */
+static void cdx_mcdi_start_or_queue(struct cdx_mcdi_iface *mcdi,
+				    bool allow_retry)
+{
+	struct cdx_mcdi_cmd *cmd, *tmp;
+
+	list_for_each_entry_safe(cmd, tmp, &mcdi->cmd_list, list)
+		if (cmd->state == MCDI_STATE_QUEUED ||
+		    (cmd->state == MCDI_STATE_RETRY && allow_retry))
+			cdx_mcdi_cmd_start_or_queue(mcdi, cmd);
+}
+
+void cdx_mcdi_process_cmd(struct cdx_mcdi *cdx, struct cdx_dword hdr)
+{
+	struct cdx_mcdi_copy_buffer *copybuf;
+	struct cdx_mcdi_iface *mcdi;
+	struct cdx_mcdi_cmd *cmd;
+	LIST_HEAD(cleanup_list);
+	unsigned int respseq;
+
+	mcdi = cdx_mcdi_if(cdx);
+	copybuf = kmalloc(sizeof(*copybuf), GFP_KERNEL);
+	respseq = CDX_DWORD_FIELD(hdr, MCDI_HEADER_SEQ);
+
+	mutex_lock(&mcdi->iface_lock);
+	cmd = mcdi->seq_held_by[respseq];
+
+	if (cmd) {
+		if (cmd->state == MCDI_STATE_FINISHED) {
+			mutex_unlock(&mcdi->iface_lock);
+			kref_put(&cmd->ref, cdx_mcdi_cmd_release);
+			kfree(copybuf);
+			return;
+		}
+
+		cdx_mcdi_complete_cmd(mcdi, cmd, copybuf, &cleanup_list);
+	} else {
+		pr_err("MC response unexpected for seq : %0X\n", respseq);
+	}
+
+	mutex_unlock(&mcdi->iface_lock);
+
+	cdx_mcdi_process_cleanup_list(mcdi->cdx, &cleanup_list);
+	kfree(copybuf);
+}
+
+static void cdx_mcdi_cmd_work(struct work_struct *context)
+{
+	struct cdx_mcdi_cmd *cmd =
+		container_of(context, struct cdx_mcdi_cmd, work);
+	struct cdx_mcdi_iface *mcdi = cmd->mcdi;
+
+	mutex_lock(&mcdi->iface_lock);
+
+	cmd->handle = mcdi->prev_handle++;
+	list_add_tail(&cmd->list, &mcdi->cmd_list);
+	cdx_mcdi_cmd_start_or_queue_ext(mcdi, cmd);
+
+	mutex_unlock(&mcdi->iface_lock);
+}
+
+/*
+ * Returns true if the MCDI module is finished with the command.
+ * (examples of false would be if the command was proxied, or it was
+ * rejected by the MC due to lack of resources and requeued).
+ */
+static bool cdx_mcdi_complete_cmd(struct cdx_mcdi_iface *mcdi,
+				  struct cdx_mcdi_cmd *cmd,
+				  struct cdx_mcdi_copy_buffer *copybuf,
+				  struct list_head *cleanup_list)
+{
+	struct cdx_dword *outbuf = copybuf ? copybuf->buffer : NULL;
+	size_t resp_hdr_len, resp_data_len;
+	struct cdx_mcdi *cdx = mcdi->cdx;
+	unsigned int respcmd, error;
+	bool completed = false;
+	u8 bufid = cmd->bufid;
+	struct cdx_dword hdr;
+	int rc;
+
+	/* ensure the command can't go away before this function returns */
+	kref_get(&cmd->ref);
+
+	cdx->mcdi_ops->mcdi_read_response(cdx, bufid, &hdr, 0, 4);
+	respcmd = CDX_DWORD_FIELD(hdr, MCDI_HEADER_CODE);
+	error = CDX_DWORD_FIELD(hdr, MCDI_HEADER_ERROR);
+
+	if (respcmd != MC_CMD_V2_EXTN) {
+		resp_hdr_len = 4;
+		resp_data_len = CDX_DWORD_FIELD(hdr, MCDI_HEADER_DATALEN);
+	} else {
+		cdx->mcdi_ops->mcdi_read_response(cdx, bufid, &hdr, 4, 4);
+		resp_hdr_len = 8;
+		resp_data_len =
+			CDX_DWORD_FIELD(hdr, MC_CMD_V2_EXTN_IN_ACTUAL_LEN);
+	}
+
+#ifdef CONFIG_MCDI_LOGGING
+	if (!WARN_ON_ONCE(!mcdi->logging_buffer)) {
+		size_t len;
+		int bytes = 0;
+		int i;
+		char *log = mcdi->logging_buffer;
+
+		WARN_ON_ONCE(resp_hdr_len % 4);
+		/*
+		 * MCDI_DECLARE_BUF ensures that underlying buffer is padded
+		 * to dword size, and the MCDI buffer is always dword size
+		 */
+		len = resp_hdr_len / 4 + DIV_ROUND_UP(resp_data_len, 4);
+
+		for (i = 0; i < len; i++) {
+			if ((bytes + 75) > LOG_LINE_MAX) {
+				pr_info("MCDI RPC RESP:%s \\\n", log);
+				bytes = 0;
+			}
+			cdx->mcdi_ops->mcdi_read_response(cdx, bufid,
+							  &hdr, (i * 4), 4);
+			bytes += snprintf(log + bytes, LOG_LINE_MAX - bytes,
+					  " %08x", le32_to_cpu(hdr.cdx_u32));
+		}
+
+		pr_info("MCDI RPC RESP:%s\n", log);
+	}
+#endif
+
+	if (error && resp_data_len == 0) {
+		/* MC rebooted during command */
+		rc = -EIO;
+	} else if (!outbuf) {
+		rc = -ENOMEM;
+	} else {
+		if (WARN_ON_ONCE(error && resp_data_len < 4))
+			resp_data_len = 4;
+
+		cdx->mcdi_ops->mcdi_read_response(cdx, bufid, outbuf,
+						  resp_hdr_len, resp_data_len);
+
+		if (error) {
+			rc = CDX_DWORD_FIELD(outbuf[0], CDX_DWORD);
+			if (!cmd->quiet) {
+				int err_arg = 0;
+
+				if (resp_data_len >= MC_CMD_ERR_ARG_OFST + 4) {
+					cdx->mcdi_ops->mcdi_read_response(cdx, bufid, &hdr,
+									  resp_hdr_len +
+									  MC_CMD_ERR_ARG_OFST, 4);
+					err_arg = CDX_DWORD_VAL(hdr);
+				}
+				_cdx_mcdi_display_error(cdx, cmd->cmd,
+							cmd->inlen, rc, err_arg,
+							cdx_mcdi_errno(cdx, rc));
+			}
+			rc = cdx_mcdi_errno(cdx, rc);
+		} else {
+			rc = 0;
+		}
+	}
+
+	/* free doorbell */
+	if (mcdi->db_held_by == cmd)
+		mcdi->db_held_by = NULL;
+
+	if (cdx_cmd_cancelled(cmd)) {
+		list_del(&cmd->list);
+		kref_put(&cmd->ref, cdx_mcdi_cmd_release);
+		completed = true;
+	} else if (rc == MC_CMD_ERR_QUEUE_FULL) {
+		cmd->state = MCDI_STATE_RETRY;
+	} else {
+		cmd->rc = rc;
+		cmd->outbuf = outbuf;
+		cmd->outlen = outbuf ? resp_data_len : 0;
+		cdx_mcdi_remove_cmd(mcdi, cmd, cleanup_list);
+		completed = true;
+	}
+
+	/* free sequence number and buffer */
+	mcdi->seq_held_by[cmd->seq] = NULL;
+	cdx->mcdi_ops->mcdi_put_buf(cdx, bufid);
+
+	cdx_mcdi_start_or_queue(mcdi, rc != MC_CMD_ERR_QUEUE_FULL);
+
+	/* wake up anyone waiting for flush */
+	wake_up(&mcdi->cmd_complete_wq);
+
+	kref_put(&cmd->ref, cdx_mcdi_cmd_release);
+
+	return completed;
+}
+
+static void cdx_mcdi_timeout_cmd(struct cdx_mcdi_iface *mcdi,
+				 struct cdx_mcdi_cmd *cmd,
+				 struct list_head *cleanup_list)
+{
+	struct cdx_mcdi *cdx = mcdi->cdx;
+
+	pr_err("MC command 0x%x inlen %zu state %d timed out after %u ms\n",
+	       cmd->cmd, cmd->inlen, cmd->state,
+	       jiffies_to_msecs(jiffies - cmd->started));
+
+	cdx->mcdi_ops->mcdi_put_buf(cdx, cmd->bufid);
+
+	cmd->rc = -ETIMEDOUT;
+	cdx_mcdi_remove_cmd(mcdi, cmd, cleanup_list);
+
+	cdx_mcdi_mode_fail(cdx, cleanup_list);
+}
+
+/**
+ * cdx_mcdi_rpc - Issue an MCDI command and wait for completion
+ * @cdx: NIC through which to issue the command
+ * @cmd: Command type number
+ * @inbuf: Command parameters
+ * @inlen: Length of command parameters, in bytes. Must be a multiple
+ *	of 4 and no greater than %MCDI_CTL_SDU_LEN_MAX_V1.
+ * @outbuf: Response buffer. May be %NULL if @outlen is 0.
+ * @outlen: Length of response buffer, in bytes. If the actual
+ *	response is longer than @outlen & ~3, it will be truncated
+ *	to that length.
+ * @outlen_actual: Pointer through which to return the actual response
+ *	length. May be %NULL if this is not needed.
+ *
+ * This function may sleep and therefore must be called in process
+ * context.
+ *
+ * @return: A negative error code, or zero if successful. The error
+ *	code may come from the MCDI response or may indicate a failure
+ *	to communicate with the MC. In the former case, the response
+ *	will still be copied to @outbuf and *@outlen_actual will be
+ *	set accordingly. In the latter case, *@outlen_actual will be
+ *	set to zero.
+ */
+int cdx_mcdi_rpc(struct cdx_mcdi *cdx, unsigned int cmd,
+		 const struct cdx_dword *inbuf, size_t inlen,
+		 struct cdx_dword *outbuf, size_t outlen,
+		 size_t *outlen_actual)
+{
+	return cdx_mcdi_rpc_sync(cdx, cmd, inbuf, inlen, outbuf, outlen,
+				 outlen_actual, false);
+}
+
+/**
+ * cdx_mcdi_rpc_async - Schedule an MCDI command to run asynchronously
+ * @cdx: NIC through which to issue the command
+ * @cmd: Command type number
+ * @inbuf: Command parameters
+ * @inlen: Length of command parameters, in bytes
+ * @outlen: Length to allocate for response buffer, in bytes
+ * @complete: Function to be called on completion or cancellation.
+ * @cookie: Arbitrary value to be passed to @complete.
+ *
+ * This function does not sleep and therefore may be called in atomic
+ * context.  It will fail if event queues are disabled or if MCDI
+ * event completions have been disabled due to an error.
+ *
+ * If it succeeds, the @complete function will be called exactly once
+ * in process context, when one of the following occurs:
+ * (a) the completion event is received (in process context)
+ * (b) event queues are disabled (in the process that disables them)
+ */
+int
+cdx_mcdi_rpc_async(struct cdx_mcdi *cdx, unsigned int cmd,
+		   const struct cdx_dword *inbuf, size_t inlen,
+		   cdx_mcdi_async_completer *complete, unsigned long cookie)
+{
+	struct cdx_mcdi_cmd *cmd_item =
+		kmalloc(sizeof(struct cdx_mcdi_cmd) + inlen, GFP_ATOMIC);
+
+	if (!cmd_item)
+		return -ENOMEM;
+
+	kref_init(&cmd_item->ref);
+	cmd_item->quiet = true;
+	cmd_item->cookie = cookie;
+	cmd_item->completer = complete;
+	cmd_item->cmd = cmd;
+	cmd_item->inlen = inlen;
+	/* inbuf is probably not valid after return, so take a copy */
+	cmd_item->inbuf = (struct cdx_dword *)(cmd_item + 1);
+	memcpy(cmd_item + 1, inbuf, inlen);
+
+	return cdx_mcdi_rpc_async_internal(cdx, cmd_item, NULL);
+}
+
+static void _cdx_mcdi_display_error(struct cdx_mcdi *cdx, unsigned int cmd,
+				    size_t inlen, int raw, int arg, int rc)
+{
+	pr_err("MC command 0x%x inlen %d failed rc=%d (raw=%d) arg=%d\n",
+	       cmd, (int)inlen, rc, raw, arg);
+}
+
+/*
+ * Set MCDI mode to fail to prevent any new commands, then cancel any
+ * outstanding commands.
+ * Caller must hold the mcdi iface_lock.
+ */
+static void cdx_mcdi_mode_fail(struct cdx_mcdi *cdx, struct list_head *cleanup_list)
+{
+	struct cdx_mcdi_iface *mcdi = cdx_mcdi_if(cdx);
+
+	if (!mcdi)
+		return;
+
+	mcdi->mode = MCDI_MODE_FAIL;
+
+	while (!list_empty(&mcdi->cmd_list)) {
+		struct cdx_mcdi_cmd *cmd;
+
+		cmd = list_first_entry(&mcdi->cmd_list, struct cdx_mcdi_cmd,
+				       list);
+		_cdx_mcdi_cancel_cmd(mcdi, cdx_mcdi_cmd_handle(cmd), cleanup_list);
+	}
+}
diff --git a/drivers/bus/cdx/controller/mcdi.h b/drivers/bus/cdx/controller/mcdi.h
new file mode 100644
index 000000000000..a2ee8821b6fe
--- /dev/null
+++ b/drivers/bus/cdx/controller/mcdi.h
@@ -0,0 +1,250 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright 2008-2013 Solarflare Communications Inc.
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+
+#ifndef CDX_MCDI_H
+#define CDX_MCDI_H
+
+#include <linux/mutex.h>
+#include <linux/kref.h>
+
+#include "bitfield.h"
+#include "mc_cdx_pcol.h"
+
+#ifdef DEBUG
+#define CDX_WARN_ON_ONCE_PARANOID(x) WARN_ON_ONCE(x)
+#define CDX_WARN_ON_PARANOID(x) WARN_ON(x)
+#else
+#define CDX_WARN_ON_ONCE_PARANOID(x) do {} while (0)
+#define CDX_WARN_ON_PARANOID(x) do {} while (0)
+#endif
+
+/**
+ * enum cdx_mcdi_mode - MCDI transaction mode
+ * @MCDI_MODE_EVENTS: wait for an mcdi response callback.
+ * @MCDI_MODE_FAIL: we think MCDI is dead, so fail-fast all calls
+ */
+enum cdx_mcdi_mode {
+	MCDI_MODE_EVENTS,
+	MCDI_MODE_FAIL,
+};
+
+#define MCDI_RPC_TIMEOUT	(10 * HZ)
+#define MCDI_RPC_LONG_TIMEOU	(60 * HZ)
+#define MCDI_RPC_POST_RST_TIME	(10 * HZ)
+
+#define MCDI_BUF_LEN (8 + MCDI_CTL_SDU_LEN_MAX)
+
+/**
+ * enum cdx_mcdi_cmd_state - State for an individual MCDI command
+ * @MCDI_STATE_QUEUED: Command not started and is waiting to run.
+ * @MCDI_STATE_RETRY: Command was submitted and MC rejected with no resources,
+ *	as MC have too many outstanding commands. Command will be retried once
+ *	another command returns.
+ * @MCDI_STATE_RUNNING: Command was accepted and is running.
+ * @MCDI_STATE_RUNNING_CANCELLED: Command is running but the issuer cancelled
+ *	the command.
+ * @MCDI_STATE_FINISHED: Processing of this command has completed.
+ */
+
+enum cdx_mcdi_cmd_state {
+	MCDI_STATE_QUEUED,
+	MCDI_STATE_RETRY,
+	MCDI_STATE_RUNNING,
+	MCDI_STATE_RUNNING_CANCELLED,
+	MCDI_STATE_FINISHED,
+};
+
+/**
+ * struct cdx_mcdi - CDX MCDI Firmware interface, to interact
+ *	with CDX controller.
+ * @mcdi: MCDI interface
+ * @mcdi_buf_use: Stores MCDI buffer usage
+ * @mcdi_ops: MCDI operations
+ * @mcdi_buf: MCDI receive buffer
+ */
+struct cdx_mcdi {
+	/* MCDI interface */
+	struct cdx_mcdi_data *mcdi;
+	unsigned long mcdi_buf_use;
+	const struct cdx_mcdi_ops *mcdi_ops;
+	void *mcdi_buf;
+};
+
+struct cdx_mcdi_ops {
+	void (*mcdi_request)(struct cdx_mcdi *cdx, u8 bufid,
+			     const struct cdx_dword *hdr, size_t hdr_len,
+			     const struct cdx_dword *sdu, size_t sdu_len);
+	bool (*mcdi_poll_response)(struct cdx_mcdi *cdx, u8 bufid);
+	void (*mcdi_read_response)(struct cdx_mcdi *cdx,
+				   u8 bufid, struct cdx_dword *pdu,
+				   size_t pdu_offset, size_t pdu_len);
+	bool (*mcdi_get_buf)(struct cdx_mcdi *cdx, u8 *bufid);
+	void (*mcdi_put_buf)(struct cdx_mcdi *cdx, u8 bufid);
+	unsigned int (*mcdi_rpc_timeout)(struct cdx_mcdi *cdx, unsigned int cmd);
+};
+
+typedef void cdx_mcdi_async_completer(struct cdx_mcdi *cdx,
+				      unsigned long cookie, int rc,
+				      struct cdx_dword *outbuf,
+				      size_t outlen_actual);
+
+/**
+ * struct cdx_mcdi_cmd - An outstanding MCDI command
+ * @ref: Reference count. There will be one reference if the command is
+ *	in the mcdi_iface cmd_list, another if it's on a cleanup list,
+ *	and a third if it's queued in the work queue.
+ * @list: The data for this entry in mcdi->cmd_list
+ * @cleanup_list: The data for this entry in a cleanup list
+ * @work: The work item for this command, queued in mcdi->workqueue
+ * @mcdi: The mcdi_iface for this command
+ * @state: The state of this command
+ * @inlen: inbuf length
+ * @inbuf: Input buffer
+ * @quiet: Whether to silence errors
+ * @reboot_seen: Whether a reboot has been seen during this command,
+ *	to prevent duplicates
+ * @seq: Sequence number
+ * @bufid: Buffer ID from the NIC implementation
+ * @started: Jiffies this command was started at
+ * @cookie: Context for completion function
+ * @completer: Completion function
+ * @handle: Command handle
+ * @cmd: Command number
+ * @rc: Return code
+ * @outlen: Length of output buffer
+ * @outbuf: Output buffer
+ */
+struct cdx_mcdi_cmd {
+	struct kref ref;
+	struct list_head list;
+	struct list_head cleanup_list;
+	struct work_struct work;
+	struct cdx_mcdi_iface *mcdi;
+	enum cdx_mcdi_cmd_state state;
+	size_t inlen;
+	const struct cdx_dword *inbuf;
+	bool quiet;
+	bool reboot_seen;
+	u8 seq;
+	u8 bufid;
+	unsigned long started;
+	unsigned long cookie;
+	cdx_mcdi_async_completer *completer;
+	unsigned int handle;
+	unsigned int cmd;
+	int rc;
+	size_t outlen;
+	struct cdx_dword *outbuf;
+	/* followed by inbuf data if necessary */
+};
+
+/**
+ * struct cdx_mcdi_iface - MCDI protocol context
+ * @cdx: The associated NIC
+ * @iface_lock: Serialise access to this structure
+ * @outstanding_cleanups: Count of cleanups
+ * @cmd_list: List of outstanding and running commands
+ * @workqueue: Workqueue used for delayed processing
+ * @cmd_complete_wq: Waitqueue for command completion
+ * @db_held_by: Command the MC doorbell is in use by
+ * @seq_held_by: Command each sequence number is in use by
+ * @prev_handle: The last used command handle
+ * @mode: Poll for mcdi completion, or wait for an mcdi_event
+ * @prev_seq: The last used sequence number
+ * @new_epoch: Indicates start of day or start of MC reboot recovery
+ * @logging_buffer: Buffer that may be used to build MCDI tracing messages
+ * @logging_enabled: Whether to trace MCDI
+ */
+struct cdx_mcdi_iface {
+	struct cdx_mcdi *cdx;
+	/* Serialise access */
+	struct mutex iface_lock;
+	unsigned int outstanding_cleanups;
+	struct list_head cmd_list;
+	struct workqueue_struct *workqueue;
+	wait_queue_head_t cmd_complete_wq;
+	struct cdx_mcdi_cmd *db_held_by;
+	struct cdx_mcdi_cmd *seq_held_by[16];
+	unsigned int prev_handle;
+	enum cdx_mcdi_mode mode;
+	u8 prev_seq;
+	bool new_epoch;
+#ifdef CONFIG_MCDI_LOGGING
+	bool logging_enabled;
+	char *logging_buffer;
+#endif
+};
+
+/**
+ * struct cdx_mcdi_data - extra state for NICs that implement MCDI
+ * @iface: Interface/protocol state
+ * @fn_flags: Flags for this function, as returned by %MC_CMD_DRV_ATTACH.
+ */
+struct cdx_mcdi_data {
+	struct cdx_mcdi_iface iface;
+	u32 fn_flags;
+};
+
+static inline struct cdx_mcdi_iface *cdx_mcdi_if(struct cdx_mcdi *cdx)
+{
+	return cdx->mcdi ? &cdx->mcdi->iface : NULL;
+}
+
+int cdx_mcdi_init(struct cdx_mcdi *cdx);
+void cdx_mcdi_finish(struct cdx_mcdi *cdx);
+
+void cdx_mcdi_process_cmd(struct cdx_mcdi *cdx, struct cdx_dword hdr);
+int cdx_mcdi_rpc(struct cdx_mcdi *cdx, unsigned int cmd,
+		 const struct cdx_dword *inbuf, size_t inlen,
+		 struct cdx_dword *outbuf, size_t outlen, size_t *outlen_actual);
+int cdx_mcdi_rpc_async(struct cdx_mcdi *cdx, unsigned int cmd,
+		       const struct cdx_dword *inbuf, size_t inlen,
+		       cdx_mcdi_async_completer *complete,
+		       unsigned long cookie);
+int cdx_mcdi_wait_for_quiescence(struct cdx_mcdi *cdx,
+				 unsigned int timeout_jiffies);
+
+/*
+ * We expect that 16- and 32-bit fields in MCDI requests and responses
+ * are appropriately aligned, but 64-bit fields are only
+ * 32-bit-aligned.
+ */
+#define MCDI_DECLARE_BUF(_name, _len) struct cdx_dword _name[DIV_ROUND_UP(_len, 4)] = {{0}}
+#define _MCDI_PTR(_buf, _offset)					\
+	((u8 *)(_buf) + (_offset))
+#define MCDI_PTR(_buf, _field)						\
+	_MCDI_PTR(_buf, MC_CMD_ ## _field ## _OFST)
+#define _MCDI_CHECK_ALIGN(_ofst, _align)				\
+	((void)BUILD_BUG_ON_ZERO((_ofst) & ((_align) - 1)),		\
+	 (_ofst))
+#define _MCDI_DWORD(_buf, _field)					\
+	((_buf) + (_MCDI_CHECK_ALIGN(MC_CMD_ ## _field ## _OFST, 4) >> 2))
+
+#define MCDI_BYTE(_buf, _field)						\
+	((void)BUILD_BUG_ON_ZERO(MC_CMD_ ## _field ## _LEN != 1),	\
+	 *MCDI_PTR(_buf, _field))
+#define MCDI_WORD(_buf, _field)						\
+	((void)BUILD_BUG_ON_ZERO(MC_CMD_ ## _field ## _LEN != 2),	\
+	 le16_to_cpu(*(__force const __le16 *)MCDI_PTR(_buf, _field)))
+#define MCDI_SET_DWORD(_buf, _field, _value)				\
+	CDX_POPULATE_DWORD_1(*_MCDI_DWORD(_buf, _field), CDX_DWORD, _value)
+#define MCDI_DWORD(_buf, _field)					\
+	CDX_DWORD_FIELD(*_MCDI_DWORD(_buf, _field), CDX_DWORD)
+#define MCDI_POPULATE_DWORD_1(_buf, _field, _name1, _value1)		\
+	CDX_POPULATE_DWORD_1(*_MCDI_DWORD(_buf, _field),		\
+			     MC_CMD_ ## _name1, _value1)
+#define MCDI_SET_QWORD(_buf, _field, _value)				\
+	do {								\
+		CDX_POPULATE_DWORD_1(_MCDI_DWORD(_buf, _field)[0],	\
+				     CDX_DWORD, (u32)(_value));	\
+		CDX_POPULATE_DWORD_1(_MCDI_DWORD(_buf, _field)[1],	\
+				     CDX_DWORD, (u64)(_value) >> 32);	\
+	} while (0)
+#define MCDI_QWORD(_buf, _field)					\
+	(CDX_DWORD_FIELD(_MCDI_DWORD(_buf, _field)[0], CDX_DWORD) |	\
+	(u64)CDX_DWORD_FIELD(_MCDI_DWORD(_buf, _field)[1], CDX_DWORD) << 32)
+
+#endif /* CDX_MCDI_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 05/19] bus/cdx: add cdx controller
  2023-01-17 13:41 [PATCH 0/7] add support for CDX bus Nipun Gupta
                   ` (3 preceding siblings ...)
  2023-01-17 13:41 ` [PATCH 04/19] bus/cdx: add MCDI protocol interface for firmware interaction Nipun Gupta
@ 2023-01-17 13:41 ` Nipun Gupta
  2023-01-17 14:10   ` Greg KH
  2023-01-17 13:41 ` [PATCH 06/19] bus/cdx: add rpmsg communication channel for CDX Nipun Gupta
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Nipun Gupta @ 2023-01-17 13:41 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, gregkh, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git, Nipun Gupta

CDX controller uses MCDI interface as a protocol to
communicate with the RPU firmware and registers the
detected CDX devices on the CDX bus. It also uses
RPMsg as the communication channel with the Firmware.

Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
Signed-off-by: Puneet Gupta <puneet.gupta@amd.com>
Signed-off-by: Nikhil Agarwal <nikhil.agarwal@amd.com>
---
 drivers/bus/cdx/controller/Kconfig          |   9 +
 drivers/bus/cdx/controller/Makefile         |   2 +-
 drivers/bus/cdx/controller/cdx_controller.c | 243 ++++++++++++++++++++
 drivers/bus/cdx/controller/mcdi_functions.c | 125 ++++++++++
 drivers/bus/cdx/controller/mcdi_functions.h |  50 ++++
 5 files changed, 428 insertions(+), 1 deletion(-)
 create mode 100644 drivers/bus/cdx/controller/cdx_controller.c
 create mode 100644 drivers/bus/cdx/controller/mcdi_functions.c
 create mode 100644 drivers/bus/cdx/controller/mcdi_functions.h

diff --git a/drivers/bus/cdx/controller/Kconfig b/drivers/bus/cdx/controller/Kconfig
index 785c71063b2a..17f9c6be2fe1 100644
--- a/drivers/bus/cdx/controller/Kconfig
+++ b/drivers/bus/cdx/controller/Kconfig
@@ -7,6 +7,15 @@
 
 if CDX_BUS
 
+config CDX_CONTROLLER
+	tristate "CDX bus controller"
+	help
+	  CDX controller drives the CDX bus. It interacts with
+	  firmware to get the hardware devices and registers with
+	  the CDX bus. Say Y to enable the CDX hardware driver.
+
+	  If unsure, say N.
+
 config MCDI_LOGGING
 	bool "MCDI Logging for the CDX controller"
 	depends on CDX_CONTROLLER
diff --git a/drivers/bus/cdx/controller/Makefile b/drivers/bus/cdx/controller/Makefile
index 0ce200678eda..f7437c882cc9 100644
--- a/drivers/bus/cdx/controller/Makefile
+++ b/drivers/bus/cdx/controller/Makefile
@@ -6,4 +6,4 @@
 #
 
 obj-$(CONFIG_CDX_CONTROLLER) += cdx-controller.o
-cdx-controller-objs := mcdi.o
+cdx-controller-objs := cdx_controller.o mcdi.o mcdi_functions.o
diff --git a/drivers/bus/cdx/controller/cdx_controller.c b/drivers/bus/cdx/controller/cdx_controller.c
new file mode 100644
index 000000000000..9b910c9cb31e
--- /dev/null
+++ b/drivers/bus/cdx/controller/cdx_controller.c
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Platform driver for CDX bus controller.
+ *
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/of_platform.h>
+#include <linux/cdx/cdx_bus.h>
+
+#include "../cdx.h"
+#include "mcdi_functions.h"
+#include "mcdi.h"
+
+#define CDX_NUM_MCDI_BUFFERS	1
+
+/*
+ * Get an MCDI buffer
+ *
+ * The caller is responsible for preventing racing by holding the
+ * MCDI iface_lock.
+ */
+static bool cdx_mcdi_get_buf(struct cdx_mcdi *cdx, u8 *bufid)
+{
+	if (!bufid)
+		return false;
+
+	*bufid = ffz(cdx->mcdi_buf_use);
+
+	if (*bufid < CDX_NUM_MCDI_BUFFERS) {
+		set_bit(*bufid, &cdx->mcdi_buf_use);
+		return true;
+	}
+
+	return false;
+}
+
+/* Return an MCDI buffer */
+static void cdx_mcdi_put_buf(struct cdx_mcdi *cdx, u8 bufid)
+{
+	CDX_WARN_ON_PARANOID(bufid >= CDX_NUM_MCDI_BUFFERS);
+	CDX_WARN_ON_PARANOID(!test_bit(bufid, &cdx->mcdi_buf_use));
+
+	clear_bit(bufid, &cdx->mcdi_buf_use);
+}
+
+static unsigned int cdx_mcdi_rpc_timeout(struct cdx_mcdi *cdx, unsigned int cmd)
+{
+	return MCDI_RPC_TIMEOUT;
+}
+
+static void cdx_mcdi_request(struct cdx_mcdi *cdx, u8 bufid,
+			     const struct cdx_dword *hdr, size_t hdr_len,
+			     const struct cdx_dword *sdu, size_t sdu_len)
+{
+	/*
+	 * This will get updated by rpmsg APIs, with RPMSG introduction
+	 * in CDX controller as a transport layer.
+	 */
+}
+
+static void cdx_mcdi_read_response(struct cdx_mcdi *cdx_mcdi, u8 bufid,
+				   struct cdx_dword *outbuf, size_t offset,
+				   size_t outlen)
+{
+	/*
+	 * This will get updated by rpmsg APIs, with RPMSG introduction
+	 * in CDX controller as a transport layer.
+	 */
+}
+
+static const struct cdx_mcdi_ops mcdi_ops = {
+	.mcdi_rpc_timeout = cdx_mcdi_rpc_timeout,
+	.mcdi_request = cdx_mcdi_request,
+	.mcdi_read_response = cdx_mcdi_read_response,
+	.mcdi_get_buf = cdx_mcdi_get_buf,
+	.mcdi_put_buf = cdx_mcdi_put_buf,
+};
+
+static int cdx_scan_devices(struct cdx_controller *cdx)
+{
+	struct cdx_mcdi *cdx_mcdi = cdx->priv;
+	u8 bus_num, dev_num, num_cdx_bus;
+	int ret;
+
+	/* MCDI FW Read: Fetch the number of CDX buses on this controller */
+	ret = cdx_mcdi_get_num_buses(cdx_mcdi);
+	if (ret < 0) {
+		dev_err(cdx->dev,
+			"Get number of CDX buses failed: %d\n", ret);
+		return ret;
+	}
+	num_cdx_bus = (u8)ret;
+
+	for (bus_num = 0; bus_num < num_cdx_bus; bus_num++) {
+		u8 num_cdx_dev;
+
+		/* MCDI FW Read: Fetch the number of devices present */
+		ret = cdx_mcdi_get_num_devs(cdx_mcdi, bus_num);
+		if (ret < 0) {
+			dev_err(cdx->dev,
+				"CDX bus %d has no devices: %d\n", bus_num, num_cdx_dev);
+			continue;
+		}
+		num_cdx_dev = (u8)ret;
+
+		for (dev_num = 0; dev_num < num_cdx_dev; dev_num++) {
+			struct cdx_dev_params dev_params;
+
+			/* MCDI FW: Get the device config */
+			ret = cdx_mcdi_get_dev_config(cdx_mcdi, bus_num,
+						      dev_num, &dev_params);
+			if (ret) {
+				dev_err(cdx->dev,
+					"CDX device config get failed for %d(bus):%d(dev), %d\n",
+					bus_num, dev_num, ret);
+				continue;
+			}
+			dev_params.cdx = cdx;
+
+			/* Add the device to the cdx bus */
+			ret = cdx_device_add(&dev_params);
+			if (ret) {
+				dev_err(cdx->dev, "registering cdx dev: %d failed: %d\n",
+					dev_num, ret);
+				continue;
+			}
+
+			dev_dbg(cdx->dev, "CDX dev: %d on cdx bus: %d created\n",
+				dev_num, bus_num);
+		}
+	}
+
+	return 0;
+}
+
+static struct cdx_ops cdx_ops = {
+	.scan		= cdx_scan_devices,
+};
+
+static int xlnx_cdx_probe(struct platform_device *pdev)
+{
+	struct cdx_controller *cdx;
+	struct cdx_mcdi *cdx_mcdi;
+	int ret;
+
+	cdx_mcdi = kzalloc(sizeof(*cdx_mcdi), GFP_KERNEL);
+	if (!cdx_mcdi)
+		return -ENOMEM;
+
+	/* Store the MCDI ops */
+	cdx_mcdi->mcdi_ops = &mcdi_ops;
+	/* MCDI FW: Initialize the FW path */
+	ret = cdx_mcdi_init(cdx_mcdi);
+	if (ret) {
+		dev_err_probe(&pdev->dev, ret, "MCDI Initialization failed\n");
+		goto mcdi_init_fail;
+	}
+
+	cdx = kzalloc(sizeof(*cdx), GFP_KERNEL);
+	if (!cdx) {
+		ret = -ENOMEM;
+		goto cdx_alloc_fail;
+	}
+	platform_set_drvdata(pdev, cdx);
+
+	cdx->dev = &pdev->dev;
+	cdx->priv = cdx_mcdi;
+	cdx->ops = &cdx_ops;
+
+	cdx_mcdi->mcdi_buf = kzalloc(MCDI_BUF_LEN, GFP_KERNEL);
+	if (!cdx_mcdi->mcdi_buf) {
+		ret = -ENOMEM;
+		goto cdx_mcdi_buf_fail;
+	}
+
+	dev_info(&pdev->dev, "Successfully registered CDX controller with RPMsg as transport\n");
+	return 0;
+
+cdx_mcdi_buf_fail:
+	kfree(cdx);
+cdx_alloc_fail:
+	cdx_mcdi_finish(cdx_mcdi);
+mcdi_init_fail:
+	kfree(cdx_mcdi);
+
+	return ret;
+}
+
+static int xlnx_cdx_remove(struct platform_device *pdev)
+{
+	struct cdx_controller *cdx = platform_get_drvdata(pdev);
+	struct cdx_mcdi *cdx_mcdi = cdx->priv;
+
+	kfree(cdx_mcdi->mcdi_buf);
+	kfree(cdx);
+
+	cdx_mcdi_finish(cdx_mcdi);
+	kfree(cdx_mcdi);
+
+	return 0;
+}
+
+static const struct of_device_id cdx_match_table[] = {
+	{.compatible = "xlnx,cdxbus-controller",},
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, cdx_match_table);
+
+static struct platform_driver cdx_pdriver = {
+	.driver = {
+		   .name = "cdx-controller",
+		   .pm = NULL,
+		   .of_match_table = cdx_match_table,
+		   },
+	.probe = xlnx_cdx_probe,
+	.remove = xlnx_cdx_remove,
+};
+
+static int __init cdx_controller_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&cdx_pdriver);
+	if (ret < 0)
+		pr_err("platform_driver_register() failed: %d\n", ret);
+
+	return ret;
+}
+
+static void __exit cdx_controller_exit(void)
+{
+	platform_driver_unregister(&cdx_pdriver);
+}
+
+module_init(cdx_controller_init);
+module_exit(cdx_controller_exit);
+
+MODULE_VERSION("1.0");
+MODULE_AUTHOR("AMD Inc.");
+MODULE_DESCRIPTION("CDX controller for AMD devices");
+MODULE_LICENSE("GPL");
diff --git a/drivers/bus/cdx/controller/mcdi_functions.c b/drivers/bus/cdx/controller/mcdi_functions.c
new file mode 100644
index 000000000000..3940a2c7919c
--- /dev/null
+++ b/drivers/bus/cdx/controller/mcdi_functions.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/module.h>
+
+#include "mcdi.h"
+#include "mcdi_functions.h"
+
+int cdx_mcdi_get_num_buses(struct cdx_mcdi *cdx)
+{
+	MCDI_DECLARE_BUF(outbuf, MC_CMD_CDX_BUS_ENUM_BUSES_OUT_LEN);
+	size_t outlen;
+	int rc;
+
+	rc = cdx_mcdi_rpc(cdx, MC_CMD_CDX_BUS_ENUM_BUSES, NULL, 0,
+			  outbuf, sizeof(outbuf), &outlen);
+	if (rc)
+		return rc;
+
+	if (outlen != MC_CMD_CDX_BUS_ENUM_BUSES_OUT_LEN)
+		return -EIO;
+
+	return MCDI_DWORD(outbuf, CDX_BUS_ENUM_BUSES_OUT_BUS_COUNT);
+}
+
+int cdx_mcdi_get_num_devs(struct cdx_mcdi *cdx, int bus_num)
+{
+	MCDI_DECLARE_BUF(outbuf, MC_CMD_CDX_BUS_ENUM_DEVICES_OUT_LEN);
+	MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_BUS_ENUM_DEVICES_IN_LEN);
+	size_t outlen;
+	int rc;
+
+	MCDI_SET_DWORD(inbuf, CDX_BUS_ENUM_DEVICES_IN_BUS, bus_num);
+
+	rc = cdx_mcdi_rpc(cdx, MC_CMD_CDX_BUS_ENUM_DEVICES, inbuf, sizeof(inbuf),
+			  outbuf, sizeof(outbuf), &outlen);
+	if (rc)
+		return rc;
+
+	if (outlen != MC_CMD_CDX_BUS_ENUM_DEVICES_OUT_LEN)
+		return -EIO;
+
+	return MCDI_DWORD(outbuf, CDX_BUS_ENUM_DEVICES_OUT_DEVICE_COUNT);
+}
+
+int cdx_mcdi_get_dev_config(struct cdx_mcdi *cdx,
+			    u8 bus_num, u8 dev_num,
+			    struct cdx_dev_params *dev_params)
+{
+	MCDI_DECLARE_BUF(outbuf, MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_LEN);
+	MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_IN_LEN);
+	struct resource *res = &dev_params->res[0];
+	size_t outlen;
+	u32 req_id;
+	int rc;
+
+	MCDI_SET_DWORD(inbuf, CDX_BUS_GET_DEVICE_CONFIG_IN_BUS, bus_num);
+	MCDI_SET_DWORD(inbuf, CDX_BUS_GET_DEVICE_CONFIG_IN_DEVICE, dev_num);
+
+	rc = cdx_mcdi_rpc(cdx, MC_CMD_CDX_BUS_GET_DEVICE_CONFIG, inbuf, sizeof(inbuf),
+			  outbuf, sizeof(outbuf), &outlen);
+	if (rc)
+		return rc;
+
+	if (outlen != MC_CMD_CDX_BUS_GET_DEVICE_CONFIG_OUT_LEN)
+		return -EIO;
+
+	dev_params->bus_num = bus_num;
+	dev_params->dev_num = dev_num;
+
+	req_id = MCDI_DWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_REQUESTER_ID);
+	dev_params->req_id = req_id;
+
+	dev_params->res_count = 0;
+	if (MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE) != 0) {
+		res[dev_params->res_count].start =
+			MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE);
+		res[dev_params->res_count].end =
+			MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_BASE) +
+				   MCDI_QWORD(outbuf,
+					      CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION0_SIZE) - 1;
+		res[dev_params->res_count].flags = IORESOURCE_MEM;
+		dev_params->res_count++;
+	}
+
+	if (MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE) != 0) {
+		res[dev_params->res_count].start =
+			MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE);
+		res[dev_params->res_count].end =
+			MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_BASE) +
+				   MCDI_QWORD(outbuf,
+					      CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION1_SIZE) - 1;
+		res[dev_params->res_count].flags = IORESOURCE_MEM;
+		dev_params->res_count++;
+	}
+
+	if (MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE) != 0) {
+		res[dev_params->res_count].start =
+			MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE);
+		res[dev_params->res_count].end =
+			MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_BASE) +
+				   MCDI_QWORD(outbuf,
+					      CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION2_SIZE) - 1;
+		res[dev_params->res_count].flags = IORESOURCE_MEM;
+		dev_params->res_count++;
+	}
+
+	if (MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE) != 0) {
+		res[dev_params->res_count].start =
+			MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE);
+		res[dev_params->res_count].end =
+			MCDI_QWORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_BASE) +
+				   MCDI_QWORD(outbuf,
+					      CDX_BUS_GET_DEVICE_CONFIG_OUT_MMIO_REGION3_SIZE) - 1;
+		res[dev_params->res_count].flags = IORESOURCE_MEM;
+		dev_params->res_count++;
+	}
+
+	dev_params->vendor = MCDI_WORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_VENDOR_ID);
+	dev_params->device = MCDI_WORD(outbuf, CDX_BUS_GET_DEVICE_CONFIG_OUT_DEVICE_ID);
+
+	return 0;
+}
diff --git a/drivers/bus/cdx/controller/mcdi_functions.h b/drivers/bus/cdx/controller/mcdi_functions.h
new file mode 100644
index 000000000000..97dfde18592f
--- /dev/null
+++ b/drivers/bus/cdx/controller/mcdi_functions.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Header file for MCDI FW interaction for CDX bus.
+ *
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+
+#ifndef CDX_MCDI_FUNCTIONS_H
+#define CDX_MCDI_FUNCTIONS_H
+
+#include "mcdi.h"
+#include "../cdx.h"
+
+/**
+ * cdx_mcdi_get_num_buses - Get the total number of buses on
+ *	the controller.
+ * @cdx: pointer to MCDI interface.
+ *
+ * @return: total number of buses available on the controller,
+ *	<0 on failure
+ */
+int cdx_mcdi_get_num_buses(struct cdx_mcdi *cdx);
+
+/**
+ * cdx_mcdi_get_num_devs - Get the total number of devices on
+ *	a particular bus of the controller.
+ * @cdx: pointer to MCDI interface.
+ * @bus_num: Bus number.
+ *
+ * @return: total number of devices available on the bus, <0 on failure
+ */
+int cdx_mcdi_get_num_devs(struct cdx_mcdi *cdx, int bus_num);
+
+/**
+ * cdx_mcdi_get_dev_config - Get configuration for a particular
+ *	bus_num:dev_num
+ * @cdx: pointer to MCDI interface.
+ * @bus_num: Bus number.
+ * @dev_num: Device number.
+ * @dev_params: Pointer to cdx_dev_params, this is populated by this
+ *	device with the configuration corresponding to the provided
+ *	bus_num:dev_num.
+ *
+ * @return: 0 total number of devices available on the bus, <0 on failure
+ */
+int cdx_mcdi_get_dev_config(struct cdx_mcdi *cdx,
+			    u8 bus_num, u8 dev_num,
+			    struct cdx_dev_params *dev_params);
+
+#endif /* CDX_MCDI_FUNCTIONS_H */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 06/19] bus/cdx: add rpmsg communication channel for CDX
  2023-01-17 13:41 [PATCH 0/7] add support for CDX bus Nipun Gupta
                   ` (4 preceding siblings ...)
  2023-01-17 13:41 ` [PATCH 05/19] bus/cdx: add cdx controller Nipun Gupta
@ 2023-01-17 13:41 ` Nipun Gupta
  2023-01-17 13:41 ` [PATCH 07/19] bus/cdx: add device attributes Nipun Gupta
  2023-01-17 17:47 ` [PATCH 0/7] add support for CDX bus Krzysztof Kozlowski
  7 siblings, 0 replies; 23+ messages in thread
From: Nipun Gupta @ 2023-01-17 13:41 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, gregkh, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git,
	Abhijit Gangurde, Nipun Gupta

From: Abhijit Gangurde <abhijit.gangurde@amd.com>

RPMsg is used as a transport communication channel. This
change introduces RPMsg driver and integrates it with the
CDX controller.

Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
---
 drivers/bus/cdx/controller/Kconfig          |   1 +
 drivers/bus/cdx/controller/Makefile         |   2 +-
 drivers/bus/cdx/controller/cdx_controller.c |  36 +++-
 drivers/bus/cdx/controller/cdx_controller.h |  30 +++
 drivers/bus/cdx/controller/cdx_rpmsg.c      | 222 ++++++++++++++++++++
 drivers/bus/cdx/controller/mcdi.h           |   9 +
 6 files changed, 291 insertions(+), 9 deletions(-)
 create mode 100644 drivers/bus/cdx/controller/cdx_controller.h
 create mode 100644 drivers/bus/cdx/controller/cdx_rpmsg.c

diff --git a/drivers/bus/cdx/controller/Kconfig b/drivers/bus/cdx/controller/Kconfig
index 17f9c6be2fe1..aea3ac86d3aa 100644
--- a/drivers/bus/cdx/controller/Kconfig
+++ b/drivers/bus/cdx/controller/Kconfig
@@ -9,6 +9,7 @@ if CDX_BUS
 
 config CDX_CONTROLLER
 	tristate "CDX bus controller"
+	select RPMSG
 	help
 	  CDX controller drives the CDX bus. It interacts with
 	  firmware to get the hardware devices and registers with
diff --git a/drivers/bus/cdx/controller/Makefile b/drivers/bus/cdx/controller/Makefile
index f7437c882cc9..f071be411d96 100644
--- a/drivers/bus/cdx/controller/Makefile
+++ b/drivers/bus/cdx/controller/Makefile
@@ -6,4 +6,4 @@
 #
 
 obj-$(CONFIG_CDX_CONTROLLER) += cdx-controller.o
-cdx-controller-objs := cdx_controller.o mcdi.o mcdi_functions.o
+cdx-controller-objs := cdx_controller.o cdx_rpmsg.o mcdi.o mcdi_functions.o
diff --git a/drivers/bus/cdx/controller/cdx_controller.c b/drivers/bus/cdx/controller/cdx_controller.c
index 9b910c9cb31e..b62a6d3b7bd4 100644
--- a/drivers/bus/cdx/controller/cdx_controller.c
+++ b/drivers/bus/cdx/controller/cdx_controller.c
@@ -8,6 +8,7 @@
 #include <linux/of_platform.h>
 #include <linux/cdx/cdx_bus.h>
 
+#include "cdx_controller.h"
 #include "../cdx.h"
 #include "mcdi_functions.h"
 #include "mcdi.h"
@@ -53,20 +54,14 @@ static void cdx_mcdi_request(struct cdx_mcdi *cdx, u8 bufid,
 			     const struct cdx_dword *hdr, size_t hdr_len,
 			     const struct cdx_dword *sdu, size_t sdu_len)
 {
-	/*
-	 * This will get updated by rpmsg APIs, with RPMSG introduction
-	 * in CDX controller as a transport layer.
-	 */
+	cdx_rpmsg_send(cdx, hdr, hdr_len, sdu, sdu_len);
 }
 
 static void cdx_mcdi_read_response(struct cdx_mcdi *cdx_mcdi, u8 bufid,
 				   struct cdx_dword *outbuf, size_t offset,
 				   size_t outlen)
 {
-	/*
-	 * This will get updated by rpmsg APIs, with RPMSG introduction
-	 * in CDX controller as a transport layer.
-	 */
+	cdx_rpmsg_read_resp(cdx_mcdi, outbuf, offset, outlen);
 }
 
 static const struct cdx_mcdi_ops mcdi_ops = {
@@ -77,6 +72,19 @@ static const struct cdx_mcdi_ops mcdi_ops = {
 	.mcdi_put_buf = cdx_mcdi_put_buf,
 };
 
+void cdx_rpmsg_post_probe(struct cdx_controller *cdx)
+{
+	/* Register CDX controller with CDX bus driver */
+	if (cdx_register_controller(cdx))
+		dev_err(cdx->dev, "Failed to register CDX controller\n");
+}
+
+void cdx_rpmsg_pre_remove(struct cdx_controller *cdx)
+{
+	cdx_unregister_controller(cdx);
+	cdx_mcdi_wait_for_quiescence(cdx->priv, MCDI_RPC_TIMEOUT);
+}
+
 static int cdx_scan_devices(struct cdx_controller *cdx)
 {
 	struct cdx_mcdi *cdx_mcdi = cdx->priv;
@@ -174,9 +182,19 @@ static int xlnx_cdx_probe(struct platform_device *pdev)
 		goto cdx_mcdi_buf_fail;
 	}
 
+	ret = cdx_setup_rpmsg(pdev);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Failed to register CDX RPMsg transport\n");
+		goto cdx_rpmsg_fail;
+	}
+
 	dev_info(&pdev->dev, "Successfully registered CDX controller with RPMsg as transport\n");
 	return 0;
 
+cdx_rpmsg_fail:
+	kfree(cdx_mcdi->mcdi_buf);
+	cdx_mcdi->mcdi_buf = NULL;
 cdx_mcdi_buf_fail:
 	kfree(cdx);
 cdx_alloc_fail:
@@ -192,6 +210,8 @@ static int xlnx_cdx_remove(struct platform_device *pdev)
 	struct cdx_controller *cdx = platform_get_drvdata(pdev);
 	struct cdx_mcdi *cdx_mcdi = cdx->priv;
 
+	cdx_destroy_rpmsg(pdev);
+
 	kfree(cdx_mcdi->mcdi_buf);
 	kfree(cdx);
 
diff --git a/drivers/bus/cdx/controller/cdx_controller.h b/drivers/bus/cdx/controller/cdx_controller.h
new file mode 100644
index 000000000000..43b7c742df87
--- /dev/null
+++ b/drivers/bus/cdx/controller/cdx_controller.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Header file for the CDX Controller
+ *
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+
+#ifndef _CDX_CONTROLLER_H_
+#define _CDX_CONTROLLER_H_
+
+#include <linux/cdx/cdx_bus.h>
+#include "mcdi_functions.h"
+
+void cdx_rpmsg_post_probe(struct cdx_controller *cdx);
+
+void cdx_rpmsg_pre_remove(struct cdx_controller *cdx);
+
+int cdx_rpmsg_send(struct cdx_mcdi *cdx_mcdi,
+		   const struct cdx_dword *hdr, size_t hdr_len,
+		   const struct cdx_dword *sdu, size_t sdu_len);
+
+void cdx_rpmsg_read_resp(struct cdx_mcdi *cdx_mcdi,
+			 struct cdx_dword *outbuf, size_t offset,
+			 size_t outlen);
+
+int cdx_setup_rpmsg(struct platform_device *pdev);
+
+void cdx_destroy_rpmsg(struct platform_device *pdev);
+
+#endif /* _CDX_CONT_PRIV_H_ */
diff --git a/drivers/bus/cdx/controller/cdx_rpmsg.c b/drivers/bus/cdx/controller/cdx_rpmsg.c
new file mode 100644
index 000000000000..147c2fadf67f
--- /dev/null
+++ b/drivers/bus/cdx/controller/cdx_rpmsg.c
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Platform driver for CDX bus.
+ *
+ * Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/rpmsg.h>
+#include <linux/remoteproc.h>
+#include <linux/of_platform.h>
+#include <linux/cdx/cdx_bus.h>
+#include <linux/module.h>
+
+#include "../cdx.h"
+#include "cdx_controller.h"
+#include "mcdi_functions.h"
+#include "mcdi.h"
+
+#define to_rproc_device(_dev) \
+	container_of(_dev, struct rproc, dev)
+
+static struct rpmsg_device_id cdx_rpmsg_id_table[] = {
+	{ .name = "mcdi_ipc" },
+	{ },
+};
+MODULE_DEVICE_TABLE(rpmsg, cdx_rpmsg_id_table);
+
+int cdx_rpmsg_send(struct cdx_mcdi *cdx_mcdi,
+		   const struct cdx_dword *hdr, size_t hdr_len,
+		   const struct cdx_dword *sdu, size_t sdu_len)
+{
+	char send_buf[MCDI_BUF_LEN] = {0};
+
+	memset(cdx_mcdi->mcdi_buf, 0x0, MCDI_BUF_LEN);
+
+	memcpy(send_buf, hdr, hdr_len);
+	memcpy(send_buf + hdr_len, sdu, sdu_len);
+
+	return rpmsg_send(cdx_mcdi->ept, send_buf, hdr_len + sdu_len);
+}
+
+void cdx_rpmsg_read_resp(struct cdx_mcdi *cdx_mcdi,
+			 struct cdx_dword *outbuf, size_t offset,
+			 size_t outlen)
+{
+	memcpy(outbuf, (void *)((u64)cdx_mcdi->mcdi_buf + offset), outlen);
+}
+
+static int find_remoteproc_child_dev(struct device *dev, void *data)
+{
+	return strstr(dev_name(dev), "remoteproc") ? 1 : 0;
+}
+
+static int cdx_attach_to_rproc(struct platform_device *pdev)
+{
+	struct platform_device *node_pdev;
+	struct device_node *r5_core_node;
+	struct cdx_controller *cdx_c;
+	struct cdx_mcdi *cdx_mcdi;
+	struct device *dev;
+	struct rproc *rp;
+	int ret;
+
+	dev = &pdev->dev;
+	cdx_c = platform_get_drvdata(pdev);
+	cdx_mcdi = cdx_c->priv;
+
+	r5_core_node = of_parse_phandle(dev->of_node, "xlnx,rproc", 0);
+	if (!r5_core_node) {
+		dev_err(&pdev->dev, "xlnx,rproc: invalid phandle\n");
+		return -EINVAL;
+	}
+
+	node_pdev = of_find_device_by_node(r5_core_node);
+	if (!node_pdev) {
+		ret = -EPROBE_DEFER;
+		goto pdev_err;
+	}
+
+	dev = device_find_child(&node_pdev->dev, NULL, find_remoteproc_child_dev);
+	if (!dev) {
+		dev_err(&pdev->dev, "no matching remoteproc device found\n");
+		ret = -ENODEV;
+		goto no_child_err;
+	}
+
+	rp = to_rproc_device(dev);
+
+	/* Attach to remote processor */
+	ret = rproc_boot(rp);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to attach to remote processor\n");
+		goto no_child_err;
+	}
+
+	cdx_mcdi->r5_rproc = rp;
+no_child_err:
+	put_device(&node_pdev->dev);
+pdev_err:
+	of_node_put(r5_core_node);
+	return ret;
+}
+
+static void cdx_detach_to_r5(struct platform_device *pdev)
+{
+	struct cdx_controller *cdx_c;
+	struct cdx_mcdi *cdx_mcdi;
+
+	cdx_c = platform_get_drvdata(pdev);
+	cdx_mcdi = cdx_c->priv;
+
+	rproc_detach(cdx_mcdi->r5_rproc);
+}
+
+static int cdx_rpmsg_cb(struct rpmsg_device *rpdev, void *data,
+			int len, void *priv, u32 src)
+{
+	struct cdx_controller *cdx_c = dev_get_drvdata(&rpdev->dev);
+	struct cdx_mcdi *cdx_mcdi = cdx_c->priv;
+
+	if (len > MCDI_BUF_LEN)
+		return -EINVAL;
+
+	memcpy(cdx_mcdi->mcdi_buf, data, len);
+	cdx_mcdi_process_cmd(cdx_mcdi, *(struct cdx_dword *)cdx_mcdi->mcdi_buf);
+
+	return 0;
+}
+
+static void cdx_rpmsg_post_probe_work(struct work_struct *work)
+{
+	struct cdx_controller *cdx_c;
+	struct cdx_mcdi *cdx_mcdi;
+
+	cdx_mcdi = container_of(work, struct cdx_mcdi, work);
+	cdx_c = dev_get_drvdata(&cdx_mcdi->rpdev->dev);
+	cdx_rpmsg_post_probe(cdx_c);
+}
+
+static int cdx_rpmsg_probe(struct rpmsg_device *rpdev)
+{
+	struct rpmsg_channel_info chinfo = {0};
+	struct cdx_controller *cdx_c;
+	struct cdx_mcdi *cdx_mcdi;
+
+	cdx_c = (struct cdx_controller *)cdx_rpmsg_id_table[0].driver_data;
+	cdx_mcdi = cdx_c->priv;
+
+	chinfo.src = RPMSG_ADDR_ANY;
+	chinfo.dst = rpdev->dst;
+	strscpy(chinfo.name, cdx_rpmsg_id_table[0].name,
+		strlen(cdx_rpmsg_id_table[0].name));
+
+	cdx_mcdi->ept = rpmsg_create_ept(rpdev, cdx_rpmsg_cb, NULL, chinfo);
+	if (!cdx_mcdi->ept) {
+		dev_err_probe(&rpdev->dev, -ENXIO,
+			      "Failed to create ept for channel %s\n",
+			      chinfo.name);
+		return -EINVAL;
+	}
+
+	cdx_mcdi->rpdev = rpdev;
+	dev_set_drvdata(&rpdev->dev, cdx_c);
+
+	schedule_work(&cdx_mcdi->work);
+	return 0;
+}
+
+static void cdx_rpmsg_remove(struct rpmsg_device *rpdev)
+{
+	struct cdx_controller *cdx_c = dev_get_drvdata(&rpdev->dev);
+	struct cdx_mcdi *cdx_mcdi = cdx_c->priv;
+
+	flush_work(&cdx_mcdi->work);
+	cdx_rpmsg_pre_remove(cdx_c);
+
+	rpmsg_destroy_ept(cdx_mcdi->ept);
+	dev_set_drvdata(&rpdev->dev, NULL);
+}
+
+static struct rpmsg_driver cdx_rpmsg_driver = {
+	.drv.name = KBUILD_MODNAME,
+	.id_table = cdx_rpmsg_id_table,
+	.probe = cdx_rpmsg_probe,
+	.remove = cdx_rpmsg_remove,
+	.callback = cdx_rpmsg_cb,
+};
+
+int cdx_setup_rpmsg(struct platform_device *pdev)
+{
+	struct cdx_controller *cdx_c;
+	struct cdx_mcdi *cdx_mcdi;
+	int ret;
+
+	/* Attach to remote processor */
+	ret = cdx_attach_to_rproc(pdev);
+	if (ret)
+		return ret;
+
+	cdx_c = platform_get_drvdata(pdev);
+	cdx_mcdi = cdx_c->priv;
+
+	/* Register RPMsg driver */
+	cdx_rpmsg_id_table[0].driver_data = (kernel_ulong_t)cdx_c;
+
+	INIT_WORK(&cdx_mcdi->work, cdx_rpmsg_post_probe_work);
+	ret = register_rpmsg_driver(&cdx_rpmsg_driver);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Failed to register cdx RPMsg driver: %d\n", ret);
+		cdx_detach_to_r5(pdev);
+	}
+
+	return ret;
+}
+
+void cdx_destroy_rpmsg(struct platform_device *pdev)
+{
+	unregister_rpmsg_driver(&cdx_rpmsg_driver);
+
+	cdx_detach_to_r5(pdev);
+}
diff --git a/drivers/bus/cdx/controller/mcdi.h b/drivers/bus/cdx/controller/mcdi.h
index a2ee8821b6fe..54e1b27c2a53 100644
--- a/drivers/bus/cdx/controller/mcdi.h
+++ b/drivers/bus/cdx/controller/mcdi.h
@@ -64,6 +64,10 @@ enum cdx_mcdi_cmd_state {
  * @mcdi_buf_use: Stores MCDI buffer usage
  * @mcdi_ops: MCDI operations
  * @mcdi_buf: MCDI receive buffer
+ * @r5_rproc : R5 Remoteproc device handle
+ * @rpdev: RPMsg device
+ * @ept: RPMsg endpoint
+ * @work: Post probe work
  */
 struct cdx_mcdi {
 	/* MCDI interface */
@@ -71,6 +75,11 @@ struct cdx_mcdi {
 	unsigned long mcdi_buf_use;
 	const struct cdx_mcdi_ops *mcdi_ops;
 	void *mcdi_buf;
+
+	struct rproc *r5_rproc;
+	struct rpmsg_device *rpdev;
+	struct rpmsg_endpoint *ept;
+	struct work_struct work;
 };
 
 struct cdx_mcdi_ops {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 07/19] bus/cdx: add device attributes
  2023-01-17 13:41 [PATCH 0/7] add support for CDX bus Nipun Gupta
                   ` (5 preceding siblings ...)
  2023-01-17 13:41 ` [PATCH 06/19] bus/cdx: add rpmsg communication channel for CDX Nipun Gupta
@ 2023-01-17 13:41 ` Nipun Gupta
  2023-01-17 14:13   ` Greg KH
  2023-01-17 17:47 ` [PATCH 0/7] add support for CDX bus Krzysztof Kozlowski
  7 siblings, 1 reply; 23+ messages in thread
From: Nipun Gupta @ 2023-01-17 13:41 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, gregkh, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git, Nipun Gupta

Create sysfs entry for CDX devices.

Sysfs entries provided in each of the CDX device detected by
the CDX controller
 - vendor id
 - device id
 - remove
 - reset of the device.
 - driver override

Signed-off-by: Puneet Gupta <puneet.gupta@amd.com>
Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
---
 Documentation/ABI/testing/sysfs-bus-cdx     |  34 +++++
 drivers/bus/cdx/cdx.c                       | 144 ++++++++++++++++++++
 drivers/bus/cdx/controller/cdx_controller.c |  19 +++
 drivers/bus/cdx/controller/mcdi_functions.c |  14 ++
 drivers/bus/cdx/controller/mcdi_functions.h |  11 ++
 include/linux/cdx/cdx_bus.h                 |  23 ++++
 6 files changed, 245 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-cdx b/Documentation/ABI/testing/sysfs-bus-cdx
index 8c2425fdb6d9..1e0fdce18cde 100644
--- a/Documentation/ABI/testing/sysfs-bus-cdx
+++ b/Documentation/ABI/testing/sysfs-bus-cdx
@@ -10,3 +10,37 @@ Description:
                 For example::
 
 		  # echo 1 > /sys/bus/cdx/rescan
+
+What:		/sys/bus/cdx/devices/.../vendor
+Date:		January 2023
+Contact:	nipun.gupta@amd.com
+Description:
+		Vendor ID for this CDX device
+
+What:		/sys/bus/cdx/devices/.../device
+Date:		January 2023
+Contact:	nipun.gupta@amd.com
+Description:
+		Device ID for this CDX device
+
+What:		/sys/bus/cdx/devices/.../reset
+Date:		January 2023
+Contact:	nipun.gupta@amd.com
+Description:
+		Writing a non-zero value to this file would reset the
+		CDX device
+
+                For example::
+
+		  # echo 1 > /sys/bus/cdx/.../reset
+
+What:		/sys/bus/cdx/devices/.../remove
+Date:		January 2023
+Contact:	tarak.reddy@amd.com
+Description:
+		Writing a non-zero value to this file would remove the
+		corrosponding device from the CDX bus
+
+		For example::
+
+		# echo 1 > /sys/bus/cdx/devices/.../remove
diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
index 2737470f08d3..1372d8dcaa4c 100644
--- a/drivers/bus/cdx/cdx.c
+++ b/drivers/bus/cdx/cdx.c
@@ -72,6 +72,38 @@ static DECLARE_BITMAP(cdx_controller_id_map, MAX_CDX_CONTROLLERS);
 /* List of CDX controllers registered with the CDX bus */
 static LIST_HEAD(cdx_controllers);
 
+/**
+ * reset_cdx_device - Reset a CDX device
+ * @dev: CDX device
+ * @data: This is always passed as NULL, and is not used in this API,
+ *	  but is required here as the bus_for_each_dev() API expects
+ *	  the passed function (reset_cdx_device) to have this
+ *	  as an argument.
+ *
+ * @return: -errno on failure, 0 on success.
+ */
+static int reset_cdx_device(struct device *dev, void *data)
+{
+	struct cdx_device *cdx_dev = to_cdx_device(dev);
+	struct cdx_controller *cdx = cdx_dev->cdx;
+	struct cdx_device_config dev_config;
+	int ret;
+
+	dev_config.type = CDX_DEV_RESET_CONF;
+	ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
+				      cdx_dev->dev_num, &dev_config);
+	if (ret)
+		dev_err(dev, "cdx device reset failed\n");
+
+	return ret;
+}
+
+int cdx_dev_reset(struct device *dev)
+{
+	return reset_cdx_device(dev, NULL);
+}
+EXPORT_SYMBOL_GPL(cdx_dev_reset);
+
 /**
  * cdx_unregister_device - Unregister a CDX device
  * @dev: CDX device
@@ -238,6 +270,117 @@ static int cdx_dma_configure(struct device *dev)
 	return 0;
 }
 
+/* show configuration fields */
+#define cdx_config_attr(field, format_string)	\
+static ssize_t	\
+field##_show(struct device *dev, struct device_attribute *attr, char *buf)	\
+{	\
+	struct cdx_device *cdx_dev = to_cdx_device(dev);	\
+	return sysfs_emit(buf, format_string, cdx_dev->field);	\
+}	\
+static DEVICE_ATTR_RO(field)
+
+cdx_config_attr(vendor, "0x%04x\n");
+cdx_config_attr(device, "0x%04x\n");
+
+static ssize_t remove_store(struct device *dev,
+			    struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	unsigned long val = 0;
+
+	if (kstrtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (!val)
+		return -EINVAL;
+
+	if (device_remove_file_self(dev, attr)) {
+		int ret;
+
+		ret = cdx_unregister_device(dev, NULL);
+		if (ret)
+			return ret;
+	}
+
+	return count;
+}
+static DEVICE_ATTR_WO(remove);
+
+static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t count)
+{
+	unsigned long val = 0;
+	int ret = 0;
+
+	if (kstrtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (!val)
+		return -EINVAL;
+
+	ret = reset_cdx_device(dev, NULL);
+	if (ret)
+		return ret;
+
+	return count;
+}
+static DEVICE_ATTR_WO(reset);
+
+static ssize_t driver_override_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct cdx_device *cdx_dev = to_cdx_device(dev);
+	const char *old = cdx_dev->driver_override;
+	char *driver_override;
+	char *cp;
+
+	if (WARN_ON(dev->bus != &cdx_bus_type))
+		return -EINVAL;
+
+	if (count >= (PAGE_SIZE - 1))
+		return -EINVAL;
+
+	driver_override = kstrndup(buf, count, GFP_KERNEL);
+	if (!driver_override)
+		return -ENOMEM;
+
+	cp = strchr(driver_override, '\n');
+	if (cp)
+		*cp = '\0';
+
+	if (strlen(driver_override)) {
+		cdx_dev->driver_override = driver_override;
+	} else {
+		kfree(driver_override);
+		cdx_dev->driver_override = NULL;
+	}
+
+	kfree(old);
+
+	return count;
+}
+
+static ssize_t driver_override_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct cdx_device *cdx_dev = to_cdx_device(dev);
+
+	return sysfs_emit(buf, "%s\n", cdx_dev->driver_override);
+}
+static DEVICE_ATTR_RW(driver_override);
+
+static struct attribute *cdx_dev_attrs[] = {
+	&dev_attr_remove.attr,
+	&dev_attr_reset.attr,
+	&dev_attr_vendor.attr,
+	&dev_attr_device.attr,
+	&dev_attr_driver_override.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(cdx_dev);
+
 static ssize_t rescan_store(struct bus_type *bus,
 			    const char *buf, size_t count)
 {
@@ -280,6 +423,7 @@ struct bus_type cdx_bus_type = {
 	.shutdown	= cdx_shutdown,
 	.dma_configure	= cdx_dma_configure,
 	.bus_groups	= cdx_bus_groups,
+	.dev_groups	= cdx_dev_groups,
 };
 EXPORT_SYMBOL_GPL(cdx_bus_type);
 
diff --git a/drivers/bus/cdx/controller/cdx_controller.c b/drivers/bus/cdx/controller/cdx_controller.c
index b62a6d3b7bd4..dbcd236b360d 100644
--- a/drivers/bus/cdx/controller/cdx_controller.c
+++ b/drivers/bus/cdx/controller/cdx_controller.c
@@ -85,6 +85,24 @@ void cdx_rpmsg_pre_remove(struct cdx_controller *cdx)
 	cdx_mcdi_wait_for_quiescence(cdx->priv, MCDI_RPC_TIMEOUT);
 }
 
+static int cdx_configure_device(struct cdx_controller *cdx,
+				u8 bus_num, u8 dev_num,
+				struct cdx_device_config *dev_config)
+{
+	int ret = 0;
+
+	switch (dev_config->type) {
+	case CDX_DEV_RESET_CONF:
+		ret = cdx_mcdi_reset_device(cdx->priv, bus_num, dev_num);
+		break;
+	default:
+		dev_err(cdx->dev, "Invalid device configuration flag\n");
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
 static int cdx_scan_devices(struct cdx_controller *cdx)
 {
 	struct cdx_mcdi *cdx_mcdi = cdx->priv;
@@ -144,6 +162,7 @@ static int cdx_scan_devices(struct cdx_controller *cdx)
 
 static struct cdx_ops cdx_ops = {
 	.scan		= cdx_scan_devices,
+	.dev_configure	= cdx_configure_device,
 };
 
 static int xlnx_cdx_probe(struct platform_device *pdev)
diff --git a/drivers/bus/cdx/controller/mcdi_functions.c b/drivers/bus/cdx/controller/mcdi_functions.c
index 3940a2c7919c..673b3896411e 100644
--- a/drivers/bus/cdx/controller/mcdi_functions.c
+++ b/drivers/bus/cdx/controller/mcdi_functions.c
@@ -123,3 +123,17 @@ int cdx_mcdi_get_dev_config(struct cdx_mcdi *cdx,
 
 	return 0;
 }
+
+int cdx_mcdi_reset_device(struct cdx_mcdi *cdx, u8 bus_num, u8 dev_num)
+{
+	MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_DEVICE_RESET_IN_LEN);
+	int rc;
+
+	MCDI_SET_DWORD(inbuf, CDX_DEVICE_RESET_IN_BUS, bus_num);
+	MCDI_SET_DWORD(inbuf, CDX_DEVICE_RESET_IN_DEVICE, dev_num);
+
+	rc = cdx_mcdi_rpc(cdx, MC_CMD_CDX_DEVICE_RESET, inbuf, sizeof(inbuf),
+			  NULL, 0, NULL);
+
+	return rc;
+}
diff --git a/drivers/bus/cdx/controller/mcdi_functions.h b/drivers/bus/cdx/controller/mcdi_functions.h
index 97dfde18592f..54d6017ab7d1 100644
--- a/drivers/bus/cdx/controller/mcdi_functions.h
+++ b/drivers/bus/cdx/controller/mcdi_functions.h
@@ -47,4 +47,15 @@ int cdx_mcdi_get_dev_config(struct cdx_mcdi *cdx,
 			    u8 bus_num, u8 dev_num,
 			    struct cdx_dev_params *dev_params);
 
+/**
+ * cdx_mcdi_reset_device - Reset cdx device represented by bus_num:dev_num
+ * @cdx: pointer to MCDI interface.
+ * @bus_num: Bus number.
+ * @dev_num: Device number.
+ *
+ * @return: 0 on success, <0 on failure
+ */
+int cdx_mcdi_reset_device(struct cdx_mcdi *cdx,
+			  u8 bus_num, u8 dev_num);
+
 #endif /* CDX_MCDI_FUNCTIONS_H */
diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
index 1d28f11da5e7..9f055739ed30 100644
--- a/include/linux/cdx/cdx_bus.h
+++ b/include/linux/cdx/cdx_bus.h
@@ -21,8 +21,20 @@
 /* Forward declaration for CDX controller */
 struct cdx_controller;
 
+enum {
+	CDX_DEV_RESET_CONF,
+};
+
+struct cdx_device_config {
+	u8 type;
+};
+
 typedef int (*cdx_scan_cb)(struct cdx_controller *cdx);
 
+typedef int (*cdx_dev_configure_cb)(struct cdx_controller *cdx,
+				    u8 bus_num, u8 dev_num,
+				    struct cdx_device_config *dev_config);
+
 /**
  * CDX_DEVICE_DRIVER_OVERRIDE - macro used to describe a CDX device with
  *                              override_only flags.
@@ -39,9 +51,12 @@ typedef int (*cdx_scan_cb)(struct cdx_controller *cdx);
 /**
  * struct cdx_ops - Callbacks supported by CDX controller.
  * @scan: scan the devices on the controller
+ * @dev_configure: configuration like reset, master_enable,
+ *		   msi_config etc for a CDX device
  */
 struct cdx_ops {
 	cdx_scan_cb scan;
+	cdx_dev_configure_cb dev_configure;
 };
 
 /**
@@ -150,4 +165,12 @@ void cdx_driver_unregister(struct cdx_driver *cdx_driver);
 
 extern struct bus_type cdx_bus_type;
 
+/**
+ * cdx_dev_reset - Reset CDX device
+ * @dev: device pointer
+ *
+ * @return: 0 for success, -errno on failure
+ */
+int cdx_dev_reset(struct device *dev);
+
 #endif /* _CDX_BUS_H_ */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/19] bus/cdx: add the cdx bus driver
  2023-01-17 13:41 ` [PATCH 01/19] bus/cdx: add the cdx bus driver Nipun Gupta
@ 2023-01-17 14:07   ` Greg KH
  2023-01-18 10:56     ` Gupta, Nipun
  2023-01-17 17:21   ` Randy Dunlap
  1 sibling, 1 reply; 23+ messages in thread
From: Greg KH @ 2023-01-17 14:07 UTC (permalink / raw)
  To: Nipun Gupta
  Cc: robh+dt, krzysztof.kozlowski+dt, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree, okaya, harpreet.anand, nikhil.agarwal, michal.simek,
	git

On Tue, Jan 17, 2023 at 07:11:33PM +0530, Nipun Gupta wrote:
> Introduce AMD CDX bus, which provides a mechanism for scanning
> and probing CDX devices. These devices are memory mapped on
> system bus for Application Processors(APUs).
> 
> CDX devices can be changed dynamically in the Fabric and CDX
> bus interacts with CDX controller to rescan the bus and
> rediscover the devices.
> 
> Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>

First off, very nice job, I didn't find any obvious issues with this
integration into the driver core.

That being said, why do you want this in drivers/bus/?  Why not
drivers/cdx/ ?

One minor comment to make the code smaller:

> +static int get_free_index(void)
> +{
> +	unsigned long id_map;
> +	unsigned long mask;
> +	int index = 0;
> +
> +	mask  = (1UL << MAX_CDX_CONTROLLERS) - 1;
> +retry:
> +	id_map = cdx_controller_id_map[0];
> +	if ((id_map & mask) == mask)
> +		return -ENOSPC;
> +
> +	index = ffz(id_map);
> +	if (index >= MAX_CDX_CONTROLLERS)
> +		return -ENOSPC;
> +
> +	if (test_and_set_bit(index, &cdx_controller_id_map[0]))
> +		goto retry;
> +
> +	return index;
> +}

Why not just use the idr/ida structure instead?  That will handle all of
that logic for you and get rid of your bit twiddling.

> +/**
> + * struct cdx_dev_params - CDX device parameters
> + * @cdx: CDX controller associated with the device
> + * @parent: Associated CDX controller
> + * @vendor: Vendor ID for CDX device
> + * @device: Device ID for CDX device
> + * @bus_num: Bus number for this CDX device
> + * @dev_num: Device number for this device
> + * @res: array of MMIO region entries
> + * @res_count: number of valid MMIO regions
> + * @req_id: Requestor ID associated with CDX device
> + */
> +struct cdx_dev_params {
> +	struct cdx_controller *cdx;
> +	u16 vendor;
> +	u16 device;

Are these in little endian format in memory?  Or native?  Or something
else?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 04/19] bus/cdx: add MCDI protocol interface for firmware interaction
  2023-01-17 13:41 ` [PATCH 04/19] bus/cdx: add MCDI protocol interface for firmware interaction Nipun Gupta
@ 2023-01-17 14:08   ` Greg KH
  2023-01-18 10:58     ` Gupta, Nipun
  0 siblings, 1 reply; 23+ messages in thread
From: Greg KH @ 2023-01-17 14:08 UTC (permalink / raw)
  To: Nipun Gupta
  Cc: robh+dt, krzysztof.kozlowski+dt, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree, okaya, harpreet.anand, nikhil.agarwal, michal.simek,
	git

On Tue, Jan 17, 2023 at 07:11:36PM +0530, Nipun Gupta wrote:
> +/** Request/Response structure */
> +#define MCDI_HEADER_OFST 0
> +#define MCDI_HEADER_CODE_LBN 0
> +#define MCDI_HEADER_CODE_WIDTH 7
> +#define MCDI_HEADER_RESYNC_LBN 7
> +#define MCDI_HEADER_RESYNC_WIDTH 1
> +#define MCDI_HEADER_DATALEN_LBN 8
> +#define MCDI_HEADER_DATALEN_WIDTH 8
> +#define MCDI_HEADER_SEQ_LBN 16
> +#define MCDI_HEADER_SEQ_WIDTH 4
> +#define MCDI_HEADER_RSVD_LBN 20
> +#define MCDI_HEADER_RSVD_WIDTH 1
> +#define MCDI_HEADER_NOT_EPOCH_LBN 21
> +#define MCDI_HEADER_NOT_EPOCH_WIDTH 1
> +#define MCDI_HEADER_ERROR_LBN 22
> +#define MCDI_HEADER_ERROR_WIDTH 1
> +#define MCDI_HEADER_RESPONSE_LBN 23
> +#define MCDI_HEADER_RESPONSE_WIDTH 1
> +#define MCDI_HEADER_XFLAGS_LBN 24
> +#define MCDI_HEADER_XFLAGS_WIDTH 8

<snip>

This whole file could use some tabs to align all of the values for the
defines to make it readable.  Any chance of doing that?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 05/19] bus/cdx: add cdx controller
  2023-01-17 13:41 ` [PATCH 05/19] bus/cdx: add cdx controller Nipun Gupta
@ 2023-01-17 14:10   ` Greg KH
  2023-01-18 12:36     ` Gupta, Nipun
  0 siblings, 1 reply; 23+ messages in thread
From: Greg KH @ 2023-01-17 14:10 UTC (permalink / raw)
  To: Nipun Gupta
  Cc: robh+dt, krzysztof.kozlowski+dt, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree, okaya, harpreet.anand, nikhil.agarwal, michal.simek,
	git

On Tue, Jan 17, 2023 at 07:11:37PM +0530, Nipun Gupta wrote:
> --- /dev/null
> +++ b/drivers/bus/cdx/controller/cdx_controller.c
> @@ -0,0 +1,243 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Platform driver for CDX bus controller.

Why is this a platform driver?  Shouldn't it also be on some type of bus
so that you can find it?

> +MODULE_VERSION("1.0");

There's never need for any module versions once the code is in the
kernel tree as then they make no sense at all.  Please drop them from
this series.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 07/19] bus/cdx: add device attributes
  2023-01-17 13:41 ` [PATCH 07/19] bus/cdx: add device attributes Nipun Gupta
@ 2023-01-17 14:13   ` Greg KH
  2023-01-18 13:03     ` Gupta, Nipun
  0 siblings, 1 reply; 23+ messages in thread
From: Greg KH @ 2023-01-17 14:13 UTC (permalink / raw)
  To: Nipun Gupta
  Cc: robh+dt, krzysztof.kozlowski+dt, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree, okaya, harpreet.anand, nikhil.agarwal, michal.simek,
	git

On Tue, Jan 17, 2023 at 07:11:39PM +0530, Nipun Gupta wrote:
> Create sysfs entry for CDX devices.
> 
> Sysfs entries provided in each of the CDX device detected by
> the CDX controller
>  - vendor id
>  - device id
>  - remove
>  - reset of the device.
>  - driver override
> 
> Signed-off-by: Puneet Gupta <puneet.gupta@amd.com>
> Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-cdx     |  34 +++++
>  drivers/bus/cdx/cdx.c                       | 144 ++++++++++++++++++++
>  drivers/bus/cdx/controller/cdx_controller.c |  19 +++
>  drivers/bus/cdx/controller/mcdi_functions.c |  14 ++
>  drivers/bus/cdx/controller/mcdi_functions.h |  11 ++
>  include/linux/cdx/cdx_bus.h                 |  23 ++++
>  6 files changed, 245 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cdx b/Documentation/ABI/testing/sysfs-bus-cdx
> index 8c2425fdb6d9..1e0fdce18cde 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cdx
> +++ b/Documentation/ABI/testing/sysfs-bus-cdx
> @@ -10,3 +10,37 @@ Description:
>                  For example::
>  
>  		  # echo 1 > /sys/bus/cdx/rescan
> +
> +What:		/sys/bus/cdx/devices/.../vendor
> +Date:		January 2023
> +Contact:	nipun.gupta@amd.com
> +Description:
> +		Vendor ID for this CDX device

What format is this in?  How big is it?  Examples?

> +
> +What:		/sys/bus/cdx/devices/.../device
> +Date:		January 2023
> +Contact:	nipun.gupta@amd.com
> +Description:
> +		Device ID for this CDX device

Same here, be specific.

> +
> +What:		/sys/bus/cdx/devices/.../reset
> +Date:		January 2023
> +Contact:	nipun.gupta@amd.com
> +Description:
> +		Writing a non-zero value to this file would reset the
> +		CDX device
> +
> +                For example::
> +
> +		  # echo 1 > /sys/bus/cdx/.../reset

Will that remove the device from the driver too?

Again, more documentation please.

> +
> +What:		/sys/bus/cdx/devices/.../remove
> +Date:		January 2023
> +Contact:	tarak.reddy@amd.com
> +Description:
> +		Writing a non-zero value to this file would remove the
> +		corrosponding device from the CDX bus
> +
> +		For example::
> +
> +		# echo 1 > /sys/bus/cdx/devices/.../remove

Why would you want to remove a device from the bus like this?

> diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
> index 2737470f08d3..1372d8dcaa4c 100644
> --- a/drivers/bus/cdx/cdx.c
> +++ b/drivers/bus/cdx/cdx.c
> @@ -72,6 +72,38 @@ static DECLARE_BITMAP(cdx_controller_id_map, MAX_CDX_CONTROLLERS);
>  /* List of CDX controllers registered with the CDX bus */
>  static LIST_HEAD(cdx_controllers);
>  
> +/**
> + * reset_cdx_device - Reset a CDX device
> + * @dev: CDX device
> + * @data: This is always passed as NULL, and is not used in this API,
> + *	  but is required here as the bus_for_each_dev() API expects
> + *	  the passed function (reset_cdx_device) to have this
> + *	  as an argument.
> + *
> + * @return: -errno on failure, 0 on success.

I recommend this function actually be the one without the data pointer,
that way you don't get confused here.

> + */
> +static int reset_cdx_device(struct device *dev, void *data)
> +{
> +	struct cdx_device *cdx_dev = to_cdx_device(dev);
> +	struct cdx_controller *cdx = cdx_dev->cdx;
> +	struct cdx_device_config dev_config;
> +	int ret;
> +
> +	dev_config.type = CDX_DEV_RESET_CONF;
> +	ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
> +				      cdx_dev->dev_num, &dev_config);
> +	if (ret)
> +		dev_err(dev, "cdx device reset failed\n");
> +
> +	return ret;
> +}
> +
> +int cdx_dev_reset(struct device *dev)
> +{
> +	return reset_cdx_device(dev, NULL);
> +}
> +EXPORT_SYMBOL_GPL(cdx_dev_reset);

Remove the indirection as mentioned above please.

Otherwise, very minor comments, nice work.

greg k-h

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/19] bus/cdx: add the cdx bus driver
  2023-01-17 13:41 ` [PATCH 01/19] bus/cdx: add the cdx bus driver Nipun Gupta
  2023-01-17 14:07   ` Greg KH
@ 2023-01-17 17:21   ` Randy Dunlap
  2023-01-18 10:59     ` Gupta, Nipun
  1 sibling, 1 reply; 23+ messages in thread
From: Randy Dunlap @ 2023-01-17 17:21 UTC (permalink / raw)
  To: Nipun Gupta, robh+dt, krzysztof.kozlowski+dt, gregkh, rafael,
	eric.auger, alex.williamson, cohuck, song.bao.hua,
	mchehab+huawei, maz, f.fainelli, jeffrey.l.hugo, saravanak,
	Michael.Srba, mani, yishaih, jgg, jgg, robin.murphy, will, joro,
	masahiroy, ndesaulniers, linux-arm-kernel, linux-kbuild,
	linux-kernel, devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git

Hi--

On 1/17/23 05:41, Nipun Gupta wrote:
> Introduce AMD CDX bus, which provides a mechanism for scanning
> and probing CDX devices. These devices are memory mapped on
> system bus for Application Processors(APUs).
> 
> CDX devices can be changed dynamically in the Fabric and CDX
> bus interacts with CDX controller to rescan the bus and
> rediscover the devices.
> 
> Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-cdx |  12 +
>  MAINTAINERS                             |   7 +
>  drivers/bus/Kconfig                     |   1 +
>  drivers/bus/Makefile                    |   2 +
>  drivers/bus/cdx/Kconfig                 |  14 +
>  drivers/bus/cdx/Makefile                |   8 +
>  drivers/bus/cdx/cdx.c                   | 433 ++++++++++++++++++++++++
>  drivers/bus/cdx/cdx.h                   |  62 ++++
>  include/linux/cdx/cdx_bus.h             | 153 +++++++++
>  include/linux/mod_devicetable.h         |  15 +
>  scripts/mod/devicetable-offsets.c       |   4 +
>  scripts/mod/file2alias.c                |  12 +
>  12 files changed, 723 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-cdx
>  create mode 100644 drivers/bus/cdx/Kconfig
>  create mode 100644 drivers/bus/cdx/Makefile
>  create mode 100644 drivers/bus/cdx/cdx.c
>  create mode 100644 drivers/bus/cdx/cdx.h
>  create mode 100644 include/linux/cdx/cdx_bus.h
> 

> diff --git a/drivers/bus/cdx/Kconfig b/drivers/bus/cdx/Kconfig
> new file mode 100644
> index 000000000000..54e0623ebcff
> --- /dev/null
> +++ b/drivers/bus/cdx/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# CDX bus configuration
> +#
> +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
> +#
> +
> +config CDX_BUS
> +	bool "CDX Bus driver"
> +	help
> +	  Driver to enable CDX Bus. CDX bus provides a mechanism for
> +	  scanning and probing of CDX devices. CDX devices are memory
> +	  mapped on system bus for embedded CPUs. CDX bus uses CDX
> +	  controller and firmware to scan the CDX devices.

Would you mind telling people who use 'make *config' what CDX means,
either in the bool prompt string or in the help text?


> +/**
> + * cdx_unregister_device - Unregister a CDX device
> + * @dev: CDX device
> + * @data: This is always passed as NULL, and is not used in this API,
> + *	  but is required here as the bus_for_each_dev() API expects
> + *	  the passed function (cdx_unregister_device) to have this
> + *	  as an argument.
> + *
> + * @return: -errno on failure, 0 on success.

The syntax (or spelling) for a function's return value in kernel-doc is just:
 * Return: -errno on failure, 0 on success.

That should be changed in many places throughout.

Thanks.

-- 
~Randy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 0/7] add support for CDX bus
  2023-01-17 13:41 [PATCH 0/7] add support for CDX bus Nipun Gupta
                   ` (6 preceding siblings ...)
  2023-01-17 13:41 ` [PATCH 07/19] bus/cdx: add device attributes Nipun Gupta
@ 2023-01-17 17:47 ` Krzysztof Kozlowski
  7 siblings, 0 replies; 23+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-17 17:47 UTC (permalink / raw)
  To: Nipun Gupta, robh+dt, krzysztof.kozlowski+dt, gregkh, rafael,
	eric.auger, alex.williamson, cohuck, song.bao.hua,
	mchehab+huawei, maz, f.fainelli, jeffrey.l.hugo, saravanak,
	Michael.Srba, mani, yishaih, jgg, jgg, robin.murphy, will, joro,
	masahiroy, ndesaulniers, linux-arm-kernel, linux-kbuild,
	linux-kernel, devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git

On 17/01/2023 14:41, Nipun Gupta wrote:
> Introduces AMD CDX bus, which provides a mechanism to
> discover/rescan CDX devices. The CDX devices are 
> memory mapped on system bus for embedded CPUs.
> 
> CDX controller interacts with the firmware to query different
> CDX devices present in the Fabric and expose them to the Linux
> host on CDX bus.
> 
> This patch series:
> - Introduces the CDX bus and CDX devices.
> - Device tree binding for CDX controller
> - Support for CDX bus in arm-smmu-v3 driver
> - Add MCDI (Management CPU Driver Interface) as a protocol
>   for communication with RPU Firmware
> - Support RPMSg channel for Firmware communication
> 
> MSI patches for CDX are not added in this series as it's
> support is being revisited as per patch series:
> https://lore.kernel.org/all/20221111133158.196269823@linutronix.de/
> It will be added as separate patches.
> 
> RFC changes with stubs were submitted at: 
> https://lore.kernel.org/linux-arm-kernel/20221014044049.2557085-1-nipun.gupta@amd.com/

This is v5 then. Please mark it appropriately, otherwise it looks like
it is first submission.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree bindings
  2023-01-17 13:41 ` [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree bindings Nipun Gupta
@ 2023-01-17 17:54   ` Krzysztof Kozlowski
  2023-01-18 12:39     ` Gupta, Nipun
  0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-17 17:54 UTC (permalink / raw)
  To: Nipun Gupta, robh+dt, krzysztof.kozlowski+dt, gregkh, rafael,
	eric.auger, alex.williamson, cohuck, song.bao.hua,
	mchehab+huawei, maz, f.fainelli, jeffrey.l.hugo, saravanak,
	Michael.Srba, mani, yishaih, jgg, jgg, robin.murphy, will, joro,
	masahiroy, ndesaulniers, linux-arm-kernel, linux-kbuild,
	linux-kernel, devicetree
  Cc: okaya, harpreet.anand, nikhil.agarwal, michal.simek, git

On 17/01/2023 14:41, Nipun Gupta wrote:
> Add device tree bindings for CDX bus controller.

Subject: drop second/last, redundant "device tree bindings". The
"dt-bindings" prefix is already stating that these are bindings.

> 
> Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> ---
>  .../bindings/bus/xlnx,cdxbus-controller.yaml  | 68 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml
> 
> diff --git a/Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml b/Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml
> new file mode 100644
> index 000000000000..b2f186864021
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/xlnx,cdxbus-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AMD CDX bus controller
> +
> +description: |
> +  CDX bus controller for AMD devices is implemented to dynamically
> +  detect CDX bus and devices on these bus using the firmware.
> +  The CDX bus manages multiple FPGA based hardware devices, which
> +  can support network, crypto or any other specialized type of
> +  devices. These FPGA based devices can be added/modified dynamically
> +  on run-time.
> +
> +  All devices on the CDX bus will have a unique streamid (for IOMMU)
> +  and a unique device ID (for MSI) corresponding to a requestor ID
> +  (one to one associated with the device). The streamid and deviceid
> +  are used to configure SMMU and GIC-ITS respectively.
> +
> +  iommu-map property is used to define the set of stream ids
> +  corresponding to each device and the associated IOMMU.
> +
> +  The MSI writes are accompanied by sideband data (Device ID).
> +  The msi-map property is used to associate the devices with the
> +  device ID as well as the associated ITS controller.
> +
> +  rproc property (xlnx,rproc) is used to identify the remote processor
> +  with which APU (Application Processor Unit) interacts to find out
> +  the bus and device configuration.
> +
> +maintainers:
> +  - Nipun Gupta <nipun.gupta@amd.com>
> +  - Nikhil Agarwal <nikhil.agarwal@amd.com>
> +
> +properties:
> +  compatible:
> +    const: xlnx,cdxbus-controller

This misses SoC specific compatible. Drop "bus" - redundant. I would
also say - drop controller - do you see any other devices with such
compatible naming? Use naming consistent with other devices in the
kernel. Just open some controllers - SPI, I2C etc. and look there.

> +
> +  iommu-map: true

No mask?

> +
> +  msi-map: true
> +
> +  xlnx,rproc:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      phandle to the remoteproc_r5 rproc node using which APU interacts
> +      with remote processor.
> +
> +required:
> +  - compatible
> +  - iommu-map
> +  - msi-map
> +  - xlnx,rproc
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    cdxbus-controller {

Node names should be generic, so just cdx.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +        compatible = "xlnx,cdxbus-controller";
> +        /* define map for RIDs 250-259 */
> +        iommu-map = <250 &smmu 250 10>;

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 01/19] bus/cdx: add the cdx bus driver
  2023-01-17 14:07   ` Greg KH
@ 2023-01-18 10:56     ` Gupta, Nipun
  0 siblings, 0 replies; 23+ messages in thread
From: Gupta, Nipun @ 2023-01-18 10:56 UTC (permalink / raw)
  To: Greg KH
  Cc: robh+dt, krzysztof.kozlowski+dt, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree, okaya, Anand, Harpreet, Agarwal, Nikhil, Simek,
	Michal, git (AMD-Xilinx)

[AMD Official Use Only - General]



> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Tuesday, January 17, 2023 7:37 PM
> To: Gupta, Nipun <Nipun.Gupta@amd.com>
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; rafael@kernel.org;
> eric.auger@redhat.com; alex.williamson@redhat.com; cohuck@redhat.com;
> song.bao.hua@hisilicon.com; mchehab+huawei@kernel.org; maz@kernel.org;
> f.fainelli@gmail.com; jeffrey.l.hugo@gmail.com; saravanak@google.com;
> Michael.Srba@seznam.cz; mani@kernel.org; yishaih@nvidia.com;
> jgg@ziepe.ca; jgg@nvidia.com; robin.murphy@arm.com; will@kernel.org;
> joro@8bytes.org; masahiroy@kernel.org; ndesaulniers@google.com; linux-arm-
> kernel@lists.infradead.org; linux-kbuild@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; okaya@kernel.org;
> Anand, Harpreet <harpreet.anand@amd.com>; Agarwal, Nikhil
> <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>; git
> (AMD-Xilinx) <git@amd.com>
> Subject: Re: [PATCH 01/19] bus/cdx: add the cdx bus driver
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> On Tue, Jan 17, 2023 at 07:11:33PM +0530, Nipun Gupta wrote:
> > Introduce AMD CDX bus, which provides a mechanism for scanning
> > and probing CDX devices. These devices are memory mapped on
> > system bus for Application Processors(APUs).
> >
> > CDX devices can be changed dynamically in the Fabric and CDX
> > bus interacts with CDX controller to rescan the bus and
> > rediscover the devices.
> >
> > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> > Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
> 
> First off, very nice job, I didn't find any obvious issues with this
> integration into the driver core.
> 
> That being said, why do you want this in drivers/bus/?  Why not
> drivers/cdx/ ?

Thanks, Greg, for taking time out for review and providing your valuable
feedback. We do not have strong affiliation to drivers/bus/cdx so will move
it to drivers/cdx in the next spin.

> 
> One minor comment to make the code smaller:
> 
> > +static int get_free_index(void)
> > +{
> > +     unsigned long id_map;
> > +     unsigned long mask;
> > +     int index = 0;
> > +
> > +     mask  = (1UL << MAX_CDX_CONTROLLERS) - 1;
> > +retry:
> > +     id_map = cdx_controller_id_map[0];
> > +     if ((id_map & mask) == mask)
> > +             return -ENOSPC;
> > +
> > +     index = ffz(id_map);
> > +     if (index >= MAX_CDX_CONTROLLERS)
> > +             return -ENOSPC;
> > +
> > +     if (test_and_set_bit(index, &cdx_controller_id_map[0]))
> > +             goto retry;
> > +
> > +     return index;
> > +}
> 
> Why not just use the idr/ida structure instead?  That will handle all of
> that logic for you and get rid of your bit twiddling.

Agree. Using idr/ida seems more appropriate. Will update the code
accordingly.

> 
> > +/**
> > + * struct cdx_dev_params - CDX device parameters
> > + * @cdx: CDX controller associated with the device
> > + * @parent: Associated CDX controller
> > + * @vendor: Vendor ID for CDX device
> > + * @device: Device ID for CDX device
> > + * @bus_num: Bus number for this CDX device
> > + * @dev_num: Device number for this device
> > + * @res: array of MMIO region entries
> > + * @res_count: number of valid MMIO regions
> > + * @req_id: Requestor ID associated with CDX device
> > + */
> > +struct cdx_dev_params {
> > +     struct cdx_controller *cdx;
> > +     u16 vendor;
> > +     u16 device;
> 
> Are these in little endian format in memory?  Or native?  Or something
> else?

While reading from the hardware, these values are little-endian; and
they are converted to CPU endianness by the controller code using
le_to_cpu32() and then passed as CPU endian while registering the
CDX device.

Thanks,
Nipun

> 
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 04/19] bus/cdx: add MCDI protocol interface for firmware interaction
  2023-01-17 14:08   ` Greg KH
@ 2023-01-18 10:58     ` Gupta, Nipun
  0 siblings, 0 replies; 23+ messages in thread
From: Gupta, Nipun @ 2023-01-18 10:58 UTC (permalink / raw)
  To: Greg KH
  Cc: robh+dt, krzysztof.kozlowski+dt, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree, okaya, Anand, Harpreet, Agarwal, Nikhil, Simek,
	Michal, git (AMD-Xilinx)

[AMD Official Use Only - General]



> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Tuesday, January 17, 2023 7:38 PM
> To: Gupta, Nipun <Nipun.Gupta@amd.com>
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; rafael@kernel.org;
> eric.auger@redhat.com; alex.williamson@redhat.com; cohuck@redhat.com;
> song.bao.hua@hisilicon.com; mchehab+huawei@kernel.org; maz@kernel.org;
> f.fainelli@gmail.com; jeffrey.l.hugo@gmail.com; saravanak@google.com;
> Michael.Srba@seznam.cz; mani@kernel.org; yishaih@nvidia.com;
> jgg@ziepe.ca; jgg@nvidia.com; robin.murphy@arm.com; will@kernel.org;
> joro@8bytes.org; masahiroy@kernel.org; ndesaulniers@google.com; linux-arm-
> kernel@lists.infradead.org; linux-kbuild@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; okaya@kernel.org;
> Anand, Harpreet <harpreet.anand@amd.com>; Agarwal, Nikhil
> <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>; git
> (AMD-Xilinx) <git@amd.com>
> Subject: Re: [PATCH 04/19] bus/cdx: add MCDI protocol interface for firmware
> interaction
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> On Tue, Jan 17, 2023 at 07:11:36PM +0530, Nipun Gupta wrote:
> > +/** Request/Response structure */
> > +#define MCDI_HEADER_OFST 0
> > +#define MCDI_HEADER_CODE_LBN 0
> > +#define MCDI_HEADER_CODE_WIDTH 7
> > +#define MCDI_HEADER_RESYNC_LBN 7
> > +#define MCDI_HEADER_RESYNC_WIDTH 1
> > +#define MCDI_HEADER_DATALEN_LBN 8
> > +#define MCDI_HEADER_DATALEN_WIDTH 8
> > +#define MCDI_HEADER_SEQ_LBN 16
> > +#define MCDI_HEADER_SEQ_WIDTH 4
> > +#define MCDI_HEADER_RSVD_LBN 20
> > +#define MCDI_HEADER_RSVD_WIDTH 1
> > +#define MCDI_HEADER_NOT_EPOCH_LBN 21
> > +#define MCDI_HEADER_NOT_EPOCH_WIDTH 1
> > +#define MCDI_HEADER_ERROR_LBN 22
> > +#define MCDI_HEADER_ERROR_WIDTH 1
> > +#define MCDI_HEADER_RESPONSE_LBN 23
> > +#define MCDI_HEADER_RESPONSE_WIDTH 1
> > +#define MCDI_HEADER_XFLAGS_LBN 24
> > +#define MCDI_HEADER_XFLAGS_WIDTH 8
> 
> <snip>
> 
> This whole file could use some tabs to align all of the values for the
> defines to make it readable.  Any chance of doing that?

Yes sure. Will update this.

> 
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 01/19] bus/cdx: add the cdx bus driver
  2023-01-17 17:21   ` Randy Dunlap
@ 2023-01-18 10:59     ` Gupta, Nipun
  0 siblings, 0 replies; 23+ messages in thread
From: Gupta, Nipun @ 2023-01-18 10:59 UTC (permalink / raw)
  To: Randy Dunlap, robh+dt, krzysztof.kozlowski+dt, gregkh, rafael,
	eric.auger, alex.williamson, cohuck, song.bao.hua,
	mchehab+huawei, maz, f.fainelli, jeffrey.l.hugo, saravanak,
	Michael.Srba, mani, yishaih, jgg, jgg, robin.murphy, will, joro,
	masahiroy, ndesaulniers, linux-arm-kernel, linux-kbuild,
	linux-kernel, devicetree
  Cc: okaya, Anand, Harpreet, Agarwal, Nikhil, Simek, Michal, git (AMD-Xilinx)

[AMD Official Use Only - General]



> -----Original Message-----
> From: Randy Dunlap <rdunlap@infradead.org>
> Sent: Tuesday, January 17, 2023 10:52 PM
> To: Gupta, Nipun <Nipun.Gupta@amd.com>; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; gregkh@linuxfoundation.org;
> rafael@kernel.org; eric.auger@redhat.com; alex.williamson@redhat.com;
> cohuck@redhat.com; song.bao.hua@hisilicon.com;
> mchehab+huawei@kernel.org; maz@kernel.org; f.fainelli@gmail.com;
> jeffrey.l.hugo@gmail.com; saravanak@google.com; Michael.Srba@seznam.cz;
> mani@kernel.org; yishaih@nvidia.com; jgg@ziepe.ca; jgg@nvidia.com;
> robin.murphy@arm.com; will@kernel.org; joro@8bytes.org;
> masahiroy@kernel.org; ndesaulniers@google.com; linux-arm-
> kernel@lists.infradead.org; linux-kbuild@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: okaya@kernel.org; Anand, Harpreet <harpreet.anand@amd.com>; Agarwal,
> Nikhil <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>;
> git (AMD-Xilinx) <git@amd.com>
> Subject: Re: [PATCH 01/19] bus/cdx: add the cdx bus driver
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> Hi--
> 
> On 1/17/23 05:41, Nipun Gupta wrote:
> > Introduce AMD CDX bus, which provides a mechanism for scanning
> > and probing CDX devices. These devices are memory mapped on
> > system bus for Application Processors(APUs).
> >
> > CDX devices can be changed dynamically in the Fabric and CDX
> > bus interacts with CDX controller to rescan the bus and
> > rediscover the devices.
> >
> > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> > Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-cdx |  12 +
> >  MAINTAINERS                             |   7 +
> >  drivers/bus/Kconfig                     |   1 +
> >  drivers/bus/Makefile                    |   2 +
> >  drivers/bus/cdx/Kconfig                 |  14 +
> >  drivers/bus/cdx/Makefile                |   8 +
> >  drivers/bus/cdx/cdx.c                   | 433 ++++++++++++++++++++++++
> >  drivers/bus/cdx/cdx.h                   |  62 ++++
> >  include/linux/cdx/cdx_bus.h             | 153 +++++++++
> >  include/linux/mod_devicetable.h         |  15 +
> >  scripts/mod/devicetable-offsets.c       |   4 +
> >  scripts/mod/file2alias.c                |  12 +
> >  12 files changed, 723 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-cdx
> >  create mode 100644 drivers/bus/cdx/Kconfig
> >  create mode 100644 drivers/bus/cdx/Makefile
> >  create mode 100644 drivers/bus/cdx/cdx.c
> >  create mode 100644 drivers/bus/cdx/cdx.h
> >  create mode 100644 include/linux/cdx/cdx_bus.h
> >
> 
> > diff --git a/drivers/bus/cdx/Kconfig b/drivers/bus/cdx/Kconfig
> > new file mode 100644
> > index 000000000000..54e0623ebcff
> > --- /dev/null
> > +++ b/drivers/bus/cdx/Kconfig
> > @@ -0,0 +1,14 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# CDX bus configuration
> > +#
> > +# Copyright (C) 2022-2023, Advanced Micro Devices, Inc.
> > +#
> > +
> > +config CDX_BUS
> > +     bool "CDX Bus driver"
> > +     help
> > +       Driver to enable CDX Bus. CDX bus provides a mechanism for
> > +       scanning and probing of CDX devices. CDX devices are memory
> > +       mapped on system bus for embedded CPUs. CDX bus uses CDX
> > +       controller and firmware to scan the CDX devices.
> 
> Would you mind telling people who use 'make *config' what CDX means,
> either in the bool prompt string or in the help text?

Sure. Will update the help section for this.

> 
> 
> > +/**
> > + * cdx_unregister_device - Unregister a CDX device
> > + * @dev: CDX device
> > + * @data: This is always passed as NULL, and is not used in this API,
> > + *     but is required here as the bus_for_each_dev() API expects
> > + *     the passed function (cdx_unregister_device) to have this
> > + *     as an argument.
> > + *
> > + * @return: -errno on failure, 0 on success.
> 
> The syntax (or spelling) for a function's return value in kernel-doc is just:
>  * Return: -errno on failure, 0 on success.
> 
> That should be changed in many places throughout.

Agree. Will update at all places.

Thanks,
Nipun

> 
> Thanks.
> 
> --
> ~Randy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 05/19] bus/cdx: add cdx controller
  2023-01-17 14:10   ` Greg KH
@ 2023-01-18 12:36     ` Gupta, Nipun
  0 siblings, 0 replies; 23+ messages in thread
From: Gupta, Nipun @ 2023-01-18 12:36 UTC (permalink / raw)
  To: Greg KH
  Cc: robh+dt, krzysztof.kozlowski+dt, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree, okaya, Anand, Harpreet, Agarwal, Nikhil, Simek,
	Michal, git (AMD-Xilinx)

[AMD Official Use Only - General]



> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Tuesday, January 17, 2023 7:40 PM
> To: Gupta, Nipun <Nipun.Gupta@amd.com>
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; rafael@kernel.org;
> eric.auger@redhat.com; alex.williamson@redhat.com; cohuck@redhat.com;
> song.bao.hua@hisilicon.com; mchehab+huawei@kernel.org; maz@kernel.org;
> f.fainelli@gmail.com; jeffrey.l.hugo@gmail.com; saravanak@google.com;
> Michael.Srba@seznam.cz; mani@kernel.org; yishaih@nvidia.com;
> jgg@ziepe.ca; jgg@nvidia.com; robin.murphy@arm.com; will@kernel.org;
> joro@8bytes.org; masahiroy@kernel.org; ndesaulniers@google.com; linux-arm-
> kernel@lists.infradead.org; linux-kbuild@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; okaya@kernel.org;
> Anand, Harpreet <harpreet.anand@amd.com>; Agarwal, Nikhil
> <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>; git
> (AMD-Xilinx) <git@amd.com>
> Subject: Re: [PATCH 05/19] bus/cdx: add cdx controller
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> On Tue, Jan 17, 2023 at 07:11:37PM +0530, Nipun Gupta wrote:
> > --- /dev/null
> > +++ b/drivers/bus/cdx/controller/cdx_controller.c
> > @@ -0,0 +1,243 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Platform driver for CDX bus controller.
> 
> Why is this a platform driver?  Shouldn't it also be on some type of bus
> so that you can find it?

This is host controller for CDX bus similar to PCI controller which is also on
the platform bus.
Since CDX bus controller is based on communication with RPU firmware we
need to have references to remoteproc device in CDX controller node to use
the RPMsg device.

> 
> > +MODULE_VERSION("1.0");
> 
> There's never need for any module versions once the code is in the
> kernel tree as then they make no sense at all.  Please drop them from
> this series.

Sure. Will remove.

> 
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree bindings
  2023-01-17 17:54   ` Krzysztof Kozlowski
@ 2023-01-18 12:39     ` Gupta, Nipun
  2023-01-18 12:43       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 23+ messages in thread
From: Gupta, Nipun @ 2023-01-18 12:39 UTC (permalink / raw)
  To: Krzysztof Kozlowski, robh+dt, krzysztof.kozlowski+dt, gregkh,
	rafael, eric.auger, alex.williamson, cohuck, song.bao.hua,
	mchehab+huawei, maz, f.fainelli, jeffrey.l.hugo, saravanak,
	Michael.Srba, mani, yishaih, jgg, jgg, robin.murphy, will, joro,
	masahiroy, ndesaulniers, linux-arm-kernel, linux-kbuild,
	linux-kernel, devicetree
  Cc: okaya, Anand, Harpreet, Agarwal, Nikhil, Simek, Michal, git (AMD-Xilinx)

[AMD Official Use Only - General]



> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Tuesday, January 17, 2023 11:24 PM
> To: Gupta, Nipun <Nipun.Gupta@amd.com>; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; gregkh@linuxfoundation.org;
> rafael@kernel.org; eric.auger@redhat.com; alex.williamson@redhat.com;
> cohuck@redhat.com; song.bao.hua@hisilicon.com;
> mchehab+huawei@kernel.org; maz@kernel.org; f.fainelli@gmail.com;
> jeffrey.l.hugo@gmail.com; saravanak@google.com; Michael.Srba@seznam.cz;
> mani@kernel.org; yishaih@nvidia.com; jgg@ziepe.ca; jgg@nvidia.com;
> robin.murphy@arm.com; will@kernel.org; joro@8bytes.org;
> masahiroy@kernel.org; ndesaulniers@google.com; linux-arm-
> kernel@lists.infradead.org; linux-kbuild@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: okaya@kernel.org; Anand, Harpreet <harpreet.anand@amd.com>; Agarwal,
> Nikhil <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>;
> git (AMD-Xilinx) <git@amd.com>
> Subject: Re: [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree
> bindings
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> On 17/01/2023 14:41, Nipun Gupta wrote:
> > Add device tree bindings for CDX bus controller.
> 
> Subject: drop second/last, redundant "device tree bindings". The
> "dt-bindings" prefix is already stating that these are bindings.

Agree. Will update.

> 
> >
> > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> > ---
> >  .../bindings/bus/xlnx,cdxbus-controller.yaml  | 68 +++++++++++++++++++
> >  MAINTAINERS                                   |  1 +
> >  2 files changed, 69 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/bus/xlnx,cdxbus-
> controller.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/bus/xlnx,cdxbus-
> controller.yaml b/Documentation/devicetree/bindings/bus/xlnx,cdxbus-
> controller.yaml
> > new file mode 100644
> > index 000000000000..b2f186864021
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/bus/xlnx,cdxbus-controller.yaml
> > @@ -0,0 +1,68 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/bus/xlnx,cdxbus-controller.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: AMD CDX bus controller
> > +
> > +description: |
> > +  CDX bus controller for AMD devices is implemented to dynamically
> > +  detect CDX bus and devices on these bus using the firmware.
> > +  The CDX bus manages multiple FPGA based hardware devices, which
> > +  can support network, crypto or any other specialized type of
> > +  devices. These FPGA based devices can be added/modified dynamically
> > +  on run-time.
> > +
> > +  All devices on the CDX bus will have a unique streamid (for IOMMU)
> > +  and a unique device ID (for MSI) corresponding to a requestor ID
> > +  (one to one associated with the device). The streamid and deviceid
> > +  are used to configure SMMU and GIC-ITS respectively.
> > +
> > +  iommu-map property is used to define the set of stream ids
> > +  corresponding to each device and the associated IOMMU.
> > +
> > +  The MSI writes are accompanied by sideband data (Device ID).
> > +  The msi-map property is used to associate the devices with the
> > +  device ID as well as the associated ITS controller.
> > +
> > +  rproc property (xlnx,rproc) is used to identify the remote processor
> > +  with which APU (Application Processor Unit) interacts to find out
> > +  the bus and device configuration.
> > +
> > +maintainers:
> > +  - Nipun Gupta <nipun.gupta@amd.com>
> > +  - Nikhil Agarwal <nikhil.agarwal@amd.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: xlnx,cdxbus-controller
> 
> This misses SoC specific compatible. Drop "bus" - redundant. I would
> also say - drop controller - do you see any other devices with such
> compatible naming? Use naming consistent with other devices in the
> kernel. Just open some controllers - SPI, I2C etc. and look there.

Makes sense. Will use "xlnx,cdx" in compatible.

> 
> > +
> > +  iommu-map: true
> 
> No mask?

Currently there is no use for iommu-map-mask as RID's are one to
one mapped stream ID's, so we have not added this optional property.

> 
> > +
> > +  msi-map: true
> > +
> > +  xlnx,rproc:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      phandle to the remoteproc_r5 rproc node using which APU interacts
> > +      with remote processor.
> > +
> > +required:
> > +  - compatible
> > +  - iommu-map
> > +  - msi-map
> > +  - xlnx,rproc
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    cdxbus-controller {
> 
> Node names should be generic, so just cdx.
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-
> basics.html#generic-names-recommendation

Sure. Will update to cdx.

Thanks,
Nipun

> 
> > +        compatible = "xlnx,cdxbus-controller";
> > +        /* define map for RIDs 250-259 */
> > +        iommu-map = <250 &smmu 250 10>;
> 
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree bindings
  2023-01-18 12:39     ` Gupta, Nipun
@ 2023-01-18 12:43       ` Krzysztof Kozlowski
  2023-01-19  7:33         ` Gupta, Nipun
  0 siblings, 1 reply; 23+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-18 12:43 UTC (permalink / raw)
  To: Gupta, Nipun, robh+dt, krzysztof.kozlowski+dt, gregkh, rafael,
	eric.auger, alex.williamson, cohuck, song.bao.hua,
	mchehab+huawei, maz, f.fainelli, jeffrey.l.hugo, saravanak,
	Michael.Srba, mani, yishaih, jgg, jgg, robin.murphy, will, joro,
	masahiroy, ndesaulniers, linux-arm-kernel, linux-kbuild,
	linux-kernel, devicetree
  Cc: okaya, Anand, Harpreet, Agarwal, Nikhil, Simek, Michal, git (AMD-Xilinx)

On 18/01/2023 13:39, Gupta, Nipun wrote:
> [AMD Official Use Only - General]

Fix your email client. This is not helping us. What shall I do with it?


> 
> 
> 

(...)


>>> +properties:
>>> +  compatible:
>>> +    const: xlnx,cdxbus-controller
>>
>> This misses SoC specific compatible. Drop "bus" - redundant. I would
>> also say - drop controller - do you see any other devices with such
>> compatible naming? Use naming consistent with other devices in the
>> kernel. Just open some controllers - SPI, I2C etc. and look there.
> 
> Makes sense. Will use "xlnx,cdx" in compatible.

No, this still misses device specific compatible. You did ignored half
of my comment now.


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 07/19] bus/cdx: add device attributes
  2023-01-17 14:13   ` Greg KH
@ 2023-01-18 13:03     ` Gupta, Nipun
  0 siblings, 0 replies; 23+ messages in thread
From: Gupta, Nipun @ 2023-01-18 13:03 UTC (permalink / raw)
  To: Greg KH
  Cc: robh+dt, krzysztof.kozlowski+dt, rafael, eric.auger,
	alex.williamson, cohuck, song.bao.hua, mchehab+huawei, maz,
	f.fainelli, jeffrey.l.hugo, saravanak, Michael.Srba, mani,
	yishaih, jgg, jgg, robin.murphy, will, joro, masahiroy,
	ndesaulniers, linux-arm-kernel, linux-kbuild, linux-kernel,
	devicetree, okaya, Anand, Harpreet, Agarwal, Nikhil, Simek,
	Michal, git (AMD-Xilinx)

[AMD Official Use Only - General]



> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Tuesday, January 17, 2023 7:44 PM
> To: Gupta, Nipun <Nipun.Gupta@amd.com>
> Cc: robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; rafael@kernel.org;
> eric.auger@redhat.com; alex.williamson@redhat.com; cohuck@redhat.com;
> song.bao.hua@hisilicon.com; mchehab+huawei@kernel.org; maz@kernel.org;
> f.fainelli@gmail.com; jeffrey.l.hugo@gmail.com; saravanak@google.com;
> Michael.Srba@seznam.cz; mani@kernel.org; yishaih@nvidia.com;
> jgg@ziepe.ca; jgg@nvidia.com; robin.murphy@arm.com; will@kernel.org;
> joro@8bytes.org; masahiroy@kernel.org; ndesaulniers@google.com; linux-arm-
> kernel@lists.infradead.org; linux-kbuild@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; okaya@kernel.org;
> Anand, Harpreet <harpreet.anand@amd.com>; Agarwal, Nikhil
> <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>; git
> (AMD-Xilinx) <git@amd.com>
> Subject: Re: [PATCH 07/19] bus/cdx: add device attributes
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> On Tue, Jan 17, 2023 at 07:11:39PM +0530, Nipun Gupta wrote:
> > Create sysfs entry for CDX devices.
> >
> > Sysfs entries provided in each of the CDX device detected by
> > the CDX controller
> >  - vendor id
> >  - device id
> >  - remove
> >  - reset of the device.
> >  - driver override
> >
> > Signed-off-by: Puneet Gupta <puneet.gupta@amd.com>
> > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com>
> > Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-cdx     |  34 +++++
> >  drivers/bus/cdx/cdx.c                       | 144 ++++++++++++++++++++
> >  drivers/bus/cdx/controller/cdx_controller.c |  19 +++
> >  drivers/bus/cdx/controller/mcdi_functions.c |  14 ++
> >  drivers/bus/cdx/controller/mcdi_functions.h |  11 ++
> >  include/linux/cdx/cdx_bus.h                 |  23 ++++
> >  6 files changed, 245 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cdx
> b/Documentation/ABI/testing/sysfs-bus-cdx
> > index 8c2425fdb6d9..1e0fdce18cde 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-cdx
> > +++ b/Documentation/ABI/testing/sysfs-bus-cdx
> > @@ -10,3 +10,37 @@ Description:
> >                  For example::
> >
> >                 # echo 1 > /sys/bus/cdx/rescan
> > +
> > +What:                /sys/bus/cdx/devices/.../vendor
> > +Date:                January 2023
> > +Contact:     nipun.gupta@amd.com
> > +Description:
> > +             Vendor ID for this CDX device
> 
> What format is this in?  How big is it?  Examples?

This is 16 bits. Agree need to add more info on this and other params.
Will update in next version.

> 
> > +
> > +What:                /sys/bus/cdx/devices/.../device
> > +Date:                January 2023
> > +Contact:     nipun.gupta@amd.com
> > +Description:
> > +             Device ID for this CDX device
> 
> Same here, be specific.
> 
> > +
> > +What:                /sys/bus/cdx/devices/.../reset
> > +Date:                January 2023
> > +Contact:     nipun.gupta@amd.com
> > +Description:
> > +             Writing a non-zero value to this file would reset the
> > +             CDX device
> > +
> > +                For example::
> > +
> > +               # echo 1 > /sys/bus/cdx/.../reset
> 
> Will that remove the device from the driver too?

No, it will not remove the device from the driver. We would
Introduce reset_done and reset_prepare callbacks which would
inform the device driver about reset in next spin.

> 
> Again, more documentation please.

Sure.

> 
> > +
> > +What:                /sys/bus/cdx/devices/.../remove
> > +Date:                January 2023
> > +Contact:     tarak.reddy@amd.com
> > +Description:
> > +             Writing a non-zero value to this file would remove the
> > +             corrosponding device from the CDX bus
> > +
> > +             For example::
> > +
> > +             # echo 1 > /sys/bus/cdx/devices/.../remove
> 
> Why would you want to remove a device from the bus like this?

This is required to prepare the bus for applying new fabric
configuration (pseudo hot plug). So that driver does not access the
device while it is being reconfigured.

> 
> > diff --git a/drivers/bus/cdx/cdx.c b/drivers/bus/cdx/cdx.c
> > index 2737470f08d3..1372d8dcaa4c 100644
> > --- a/drivers/bus/cdx/cdx.c
> > +++ b/drivers/bus/cdx/cdx.c
> > @@ -72,6 +72,38 @@ static DECLARE_BITMAP(cdx_controller_id_map,
> MAX_CDX_CONTROLLERS);
> >  /* List of CDX controllers registered with the CDX bus */
> >  static LIST_HEAD(cdx_controllers);
> >
> > +/**
> > + * reset_cdx_device - Reset a CDX device
> > + * @dev: CDX device
> > + * @data: This is always passed as NULL, and is not used in this API,
> > + *     but is required here as the bus_for_each_dev() API expects
> > + *     the passed function (reset_cdx_device) to have this
> > + *     as an argument.
> > + *
> > + * @return: -errno on failure, 0 on success.
> 
> I recommend this function actually be the one without the data pointer,
> that way you don't get confused here.

Agree will update this.

> 
> > + */
> > +static int reset_cdx_device(struct device *dev, void *data)
> > +{
> > +     struct cdx_device *cdx_dev = to_cdx_device(dev);
> > +     struct cdx_controller *cdx = cdx_dev->cdx;
> > +     struct cdx_device_config dev_config;
> > +     int ret;
> > +
> > +     dev_config.type = CDX_DEV_RESET_CONF;
> > +     ret = cdx->ops->dev_configure(cdx, cdx_dev->bus_num,
> > +                                   cdx_dev->dev_num, &dev_config);
> > +     if (ret)
> > +             dev_err(dev, "cdx device reset failed\n");
> > +
> > +     return ret;
> > +}
> > +
> > +int cdx_dev_reset(struct device *dev)
> > +{
> > +     return reset_cdx_device(dev, NULL);
> > +}
> > +EXPORT_SYMBOL_GPL(cdx_dev_reset);
> 
> Remove the indirection as mentioned above please.
> 
> Otherwise, very minor comments, nice work.
> 
> greg k-h

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree bindings
  2023-01-18 12:43       ` Krzysztof Kozlowski
@ 2023-01-19  7:33         ` Gupta, Nipun
  0 siblings, 0 replies; 23+ messages in thread
From: Gupta, Nipun @ 2023-01-19  7:33 UTC (permalink / raw)
  To: Krzysztof Kozlowski, robh+dt, krzysztof.kozlowski+dt, gregkh,
	rafael, eric.auger, alex.williamson, cohuck, song.bao.hua,
	mchehab+huawei, maz, f.fainelli, jeffrey.l.hugo, saravanak,
	Michael.Srba, mani, yishaih, jgg, jgg, robin.murphy, will, joro,
	masahiroy, ndesaulniers, linux-arm-kernel, linux-kbuild,
	linux-kernel, devicetree
  Cc: okaya, Anand, Harpreet, Agarwal, Nikhil, Simek, Michal, git (AMD-Xilinx)



> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Wednesday, January 18, 2023 6:13 PM
> To: Gupta, Nipun <Nipun.Gupta@amd.com>; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; gregkh@linuxfoundation.org;
> rafael@kernel.org; eric.auger@redhat.com; alex.williamson@redhat.com;
> cohuck@redhat.com; song.bao.hua@hisilicon.com;
> mchehab+huawei@kernel.org; maz@kernel.org; f.fainelli@gmail.com;
> jeffrey.l.hugo@gmail.com; saravanak@google.com; Michael.Srba@seznam.cz;
> mani@kernel.org; yishaih@nvidia.com; jgg@ziepe.ca; jgg@nvidia.com;
> robin.murphy@arm.com; will@kernel.org; joro@8bytes.org;
> masahiroy@kernel.org; ndesaulniers@google.com; linux-arm-
> kernel@lists.infradead.org; linux-kbuild@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: okaya@kernel.org; Anand, Harpreet <harpreet.anand@amd.com>; Agarwal,
> Nikhil <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>;
> git (AMD-Xilinx) <git@amd.com>
> Subject: Re: [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree
> bindings

(...)

> >>> +properties:
> >>> +  compatible:
> >>> +    const: xlnx,cdxbus-controller
> >>
> >> This misses SoC specific compatible. Drop "bus" - redundant. I would
> >> also say - drop controller - do you see any other devices with such
> >> compatible naming? Use naming consistent with other devices in the
> >> kernel. Just open some controllers - SPI, I2C etc. and look there.
> >
> > Makes sense. Will use "xlnx,cdx" in compatible.
> 
> No, this still misses device specific compatible. You did ignored half
> of my comment now.

Sorry I missed that part. Will rename this to "xlnx,versal-cdx".

Thanks,
Nipun

> 
> 
> Best regards,
> Krzysztof


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2023-01-19  7:33 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-17 13:41 [PATCH 0/7] add support for CDX bus Nipun Gupta
2023-01-17 13:41 ` [PATCH 01/19] bus/cdx: add the cdx bus driver Nipun Gupta
2023-01-17 14:07   ` Greg KH
2023-01-18 10:56     ` Gupta, Nipun
2023-01-17 17:21   ` Randy Dunlap
2023-01-18 10:59     ` Gupta, Nipun
2023-01-17 13:41 ` [PATCH 02/19] iommu/arm-smmu-v3: support ops registration for CDX bus Nipun Gupta
2023-01-17 13:41 ` [PATCH 03/19] dt-bindings: bus: add CDX bus controller device tree bindings Nipun Gupta
2023-01-17 17:54   ` Krzysztof Kozlowski
2023-01-18 12:39     ` Gupta, Nipun
2023-01-18 12:43       ` Krzysztof Kozlowski
2023-01-19  7:33         ` Gupta, Nipun
2023-01-17 13:41 ` [PATCH 04/19] bus/cdx: add MCDI protocol interface for firmware interaction Nipun Gupta
2023-01-17 14:08   ` Greg KH
2023-01-18 10:58     ` Gupta, Nipun
2023-01-17 13:41 ` [PATCH 05/19] bus/cdx: add cdx controller Nipun Gupta
2023-01-17 14:10   ` Greg KH
2023-01-18 12:36     ` Gupta, Nipun
2023-01-17 13:41 ` [PATCH 06/19] bus/cdx: add rpmsg communication channel for CDX Nipun Gupta
2023-01-17 13:41 ` [PATCH 07/19] bus/cdx: add device attributes Nipun Gupta
2023-01-17 14:13   ` Greg KH
2023-01-18 13:03     ` Gupta, Nipun
2023-01-17 17:47 ` [PATCH 0/7] add support for CDX bus Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).