2023-01-17 14:05:34

by Nipun Gupta

[permalink] [raw]
Subject: [PATCH 05/19] bus/cdx: add cdx controller

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 <[email protected]>
Signed-off-by: Puneet Gupta <[email protected]>
Signed-off-by: Nikhil Agarwal <[email protected]>
---
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


2023-01-17 14:17:28

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 05/19] bus/cdx: add cdx controller

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

2023-01-18 13:34:34

by Nipun Gupta

[permalink] [raw]
Subject: RE: [PATCH 05/19] bus/cdx: add cdx controller

[AMD Official Use Only - General]



> -----Original Message-----
> From: Greg KH <[email protected]>
> Sent: Tuesday, January 17, 2023 7:40 PM
> To: Gupta, Nipun <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> Anand, Harpreet <[email protected]>; Agarwal, Nikhil
> <[email protected]>; Simek, Michal <[email protected]>; git
> (AMD-Xilinx) <[email protected]>
> 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