[PATCH v6 4/6] cdx: add support for bus enable and disable
From: Abhijit Gangurde
Date: Fri Oct 13 2023 - 02:10:43 EST
CDX bus needs to be disabled before updating/writing devices
in the FPGA. Once the devices are written, the bus shall be
rescanned. This change provides sysfs entry to enable/disable the
CDX bus.
Co-developed-by: Nipun Gupta <nipun.gupta@xxxxxxx>
Signed-off-by: Nipun Gupta <nipun.gupta@xxxxxxx>
Signed-off-by: Abhijit Gangurde <abhijit.gangurde@xxxxxxx>
---
Changes in v6:
- Remove bus attributes enable and disable
- Add enable attribute for cdx device visible only for bus device.
Changes in v5:
- Rebased on Linux 6.6
Changes in v4:
- Add sparse marking for lock around cdx_find_controller() function
Changes in v3:
- Use platform bus to locate controller
Changes in v2:
- Improve documentation
- Split sysfs entry enable to enable and disable
- sysfs entry enable and disable take bus number as an argument
Documentation/ABI/testing/sysfs-bus-cdx | 13 +++++
drivers/cdx/cdx.c | 72 +++++++++++++++++++++++++
drivers/cdx/controller/cdx_controller.c | 12 +++++
drivers/cdx/controller/mc_cdx_pcol.h | 54 +++++++++++++++++++
drivers/cdx/controller/mcdi_functions.c | 24 +++++++++
drivers/cdx/controller/mcdi_functions.h | 18 +++++++
include/linux/cdx/cdx_bus.h | 10 ++++
7 files changed, 203 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-cdx b/Documentation/ABI/testing/sysfs-bus-cdx
index 7af477f49998..c12bdaa4152a 100644
--- a/Documentation/ABI/testing/sysfs-bus-cdx
+++ b/Documentation/ABI/testing/sysfs-bus-cdx
@@ -28,6 +28,19 @@ Description:
of a device manufacturer.
Combination of Vendor ID and Device ID identifies a device.
+What: /sys/bus/cdx/devices/.../enable
+Date: October 2023
+Contact: abhijit.gangurde@xxxxxxx
+Description:
+ CDX bus should be disabled before updating the devices in FPGA.
+ Writing n/0/off will attempt to disable the CDX bus and.
+ writing y/1/on will attempt to enable the CDX bus. Reading this file
+ gives the current state of the bus, 1 for enabled and 0 for disabled.
+
+ For example::
+
+ # echo 1 > /sys/bus/cdx/.../enable
+
What: /sys/bus/cdx/devices/.../reset
Date: March 2023
Contact: nipun.gupta@xxxxxxx
diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
index 18da06e0babe..3d7bdd2647a3 100644
--- a/drivers/cdx/cdx.c
+++ b/drivers/cdx/cdx.c
@@ -124,9 +124,12 @@ static int cdx_unregister_device(struct device *dev,
void *data)
{
struct cdx_device *cdx_dev = to_cdx_device(dev);
+ struct cdx_controller *cdx = cdx_dev->cdx;
if (cdx_dev->is_bus) {
device_for_each_child(dev, NULL, cdx_unregister_device);
+ if (cdx_dev->enabled && cdx->ops->bus_disable)
+ cdx->ops->bus_disable(cdx, cdx_dev->bus_num);
} else {
kfree(cdx_dev->driver_override);
cdx_dev->driver_override = NULL;
@@ -383,6 +386,41 @@ static ssize_t driver_override_show(struct device *dev,
}
static DEVICE_ATTR_RW(driver_override);
+static ssize_t enable_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct cdx_device *cdx_dev = to_cdx_device(dev);
+ struct cdx_controller *cdx = cdx_dev->cdx;
+ bool enable;
+ int ret;
+
+ if (kstrtobool(buf, &enable) < 0)
+ return -EINVAL;
+
+ if (enable == cdx_dev->enabled)
+ return count;
+
+ if (enable && cdx->ops->bus_enable)
+ ret = cdx->ops->bus_enable(cdx, cdx_dev->bus_num);
+ else if (!enable && cdx->ops->bus_disable)
+ ret = cdx->ops->bus_disable(cdx, cdx_dev->bus_num);
+ else
+ ret = -EOPNOTSUPP;
+
+ if (!ret)
+ cdx_dev->enabled = enable;
+
+ return ret < 0 ? ret : count;
+}
+
+static ssize_t enable_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct cdx_device *cdx_dev = to_cdx_device(dev);
+
+ return sysfs_emit(buf, "%u\n", cdx_dev->enabled);
+}
+static DEVICE_ATTR_RW(enable);
+
static umode_t cdx_dev_attrs_are_visible(struct kobject *kobj, struct attribute *a, int n)
{
struct device *dev = kobj_to_dev(kobj);
@@ -395,6 +433,18 @@ static umode_t cdx_dev_attrs_are_visible(struct kobject *kobj, struct attribute
return 0;
}
+static umode_t cdx_bus_attrs_are_visible(struct kobject *kobj, struct attribute *a, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct cdx_device *cdx_dev;
+
+ cdx_dev = to_cdx_device(dev);
+ if (cdx_dev->is_bus)
+ return a->mode;
+
+ return 0;
+}
+
static struct attribute *cdx_dev_attrs[] = {
&dev_attr_remove.attr,
&dev_attr_reset.attr,
@@ -409,8 +459,19 @@ static const struct attribute_group cdx_dev_group = {
.is_visible = cdx_dev_attrs_are_visible,
};
+static struct attribute *cdx_bus_dev_attrs[] = {
+ &dev_attr_enable.attr,
+ NULL,
+};
+
+static const struct attribute_group cdx_bus_dev_group = {
+ .attrs = cdx_bus_dev_attrs,
+ .is_visible = cdx_bus_attrs_are_visible,
+};
+
static const struct attribute_group *cdx_dev_groups[] = {
&cdx_dev_group,
+ &cdx_bus_dev_group,
NULL,
};
@@ -588,8 +649,19 @@ struct device *cdx_bus_add(struct cdx_controller *cdx, u8 bus_num)
goto device_add_fail;
}
+ if (cdx->ops->bus_enable) {
+ ret = cdx->ops->bus_enable(cdx, bus_num);
+ if (ret && ret != -EALREADY) {
+ dev_err(cdx->dev, "cdx bus enable failed: %d\n", ret);
+ goto bus_enable_fail;
+ }
+ }
+
+ cdx_dev->enabled = true;
return &cdx_dev->dev;
+bus_enable_fail:
+ device_del(&cdx_dev->dev);
device_add_fail:
put_device(&cdx_dev->dev);
diff --git a/drivers/cdx/controller/cdx_controller.c b/drivers/cdx/controller/cdx_controller.c
index d1d4606f6c1a..b818f9815ea8 100644
--- a/drivers/cdx/controller/cdx_controller.c
+++ b/drivers/cdx/controller/cdx_controller.c
@@ -33,6 +33,16 @@ static const struct cdx_mcdi_ops mcdi_ops = {
.mcdi_request = cdx_mcdi_request,
};
+static int cdx_bus_enable(struct cdx_controller *cdx, u8 bus_num)
+{
+ return cdx_mcdi_bus_enable(cdx->priv, bus_num);
+}
+
+static int cdx_bus_disable(struct cdx_controller *cdx, u8 bus_num)
+{
+ return cdx_mcdi_bus_disable(cdx->priv, bus_num);
+}
+
void cdx_rpmsg_post_probe(struct cdx_controller *cdx)
{
/* Register CDX controller with CDX bus driver */
@@ -128,6 +138,8 @@ static int cdx_scan_devices(struct cdx_controller *cdx)
}
static struct cdx_ops cdx_ops = {
+ .bus_enable = cdx_bus_enable,
+ .bus_disable = cdx_bus_disable,
.scan = cdx_scan_devices,
.dev_configure = cdx_configure_device,
};
diff --git a/drivers/cdx/controller/mc_cdx_pcol.h b/drivers/cdx/controller/mc_cdx_pcol.h
index 4ccb7b52951b..2de019406b57 100644
--- a/drivers/cdx/controller/mc_cdx_pcol.h
+++ b/drivers/cdx/controller/mc_cdx_pcol.h
@@ -455,6 +455,60 @@
#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_BUS_DOWN
+ * Asserting reset on the CDX bus causes all devices on the bus to be quiesced.
+ * DMA bus mastering is disabled and any pending DMA request are flushed. Once
+ * the response is returned, the devices are guaranteed to no longer issue DMA
+ * requests or raise MSI interrupts. Further device MMIO accesses may have
+ * undefined results. While the bus reset is asserted, any of the enumeration
+ * or device configuration MCDIs will fail with EAGAIN. It is only legal to
+ * reload the relevant PL region containing CDX devices if the corresponding CDX
+ * bus is in reset. Depending on the implementation, the firmware may or may
+ * not enforce this restriction and it is up to the caller to make sure this
+ * requirement is satisfied.
+ */
+#define MC_CMD_CDX_BUS_DOWN 0x4
+#define MC_CMD_CDX_BUS_DOWN_MSGSET 0x4
+
+/* MC_CMD_CDX_BUS_DOWN_IN msgrequest */
+#define MC_CMD_CDX_BUS_DOWN_IN_LEN 4
+/* Bus number to put in reset, in range 0 to BUS_COUNT-1 */
+#define MC_CMD_CDX_BUS_DOWN_IN_BUS_OFST 0
+#define MC_CMD_CDX_BUS_DOWN_IN_BUS_LEN 4
+
+/*
+ * MC_CMD_CDX_BUS_DOWN_OUT msgresponse: The bus is quiesced, no further
+ * upstream traffic for devices on this bus.
+ */
+#define MC_CMD_CDX_BUS_DOWN_OUT_LEN 0
+
+/***********************************/
+/*
+ * MC_CMD_CDX_BUS_UP
+ * After bus reset is de-asserted, devices are in a state which is functionally
+ * equivalent to each device having been reset with MC_CMD_CDX_DEVICE_RESET. In
+ * other words, device logic is reset in a hardware-specific way, MMIO accesses
+ * are forwarded to the device, DMA bus mastering is disabled and needs to be
+ * re-enabled with MC_CMD_CDX_DEVICE_DMA_ENABLE once the driver is ready to
+ * start servicing DMA. If the underlying number of devices or device resources
+ * changed (e.g. if PL was reloaded) while the bus was in reset, the bus driver
+ * is expected to re-enumerate the bus. Returns EALREADY if the bus was already
+ * up before the call.
+ */
+#define MC_CMD_CDX_BUS_UP 0x5
+#define MC_CMD_CDX_BUS_UP_MSGSET 0x5
+
+/* MC_CMD_CDX_BUS_UP_IN msgrequest */
+#define MC_CMD_CDX_BUS_UP_IN_LEN 4
+/* Bus number to take out of reset, in range 0 to BUS_COUNT-1 */
+#define MC_CMD_CDX_BUS_UP_IN_BUS_OFST 0
+#define MC_CMD_CDX_BUS_UP_IN_BUS_LEN 4
+
+/* MC_CMD_CDX_BUS_UP_OUT msgresponse: The bus can now be enumerated. */
+#define MC_CMD_CDX_BUS_UP_OUT_LEN 0
+
/***********************************/
/*
* MC_CMD_CDX_DEVICE_RESET
diff --git a/drivers/cdx/controller/mcdi_functions.c b/drivers/cdx/controller/mcdi_functions.c
index 0158f26533dd..0e1e35d91242 100644
--- a/drivers/cdx/controller/mcdi_functions.c
+++ b/drivers/cdx/controller/mcdi_functions.c
@@ -124,6 +124,30 @@ int cdx_mcdi_get_dev_config(struct cdx_mcdi *cdx,
return 0;
}
+int cdx_mcdi_bus_enable(struct cdx_mcdi *cdx, u8 bus_num)
+{
+ MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_BUS_UP_IN_LEN);
+ int ret;
+
+ MCDI_SET_DWORD(inbuf, CDX_BUS_UP_IN_BUS, bus_num);
+ ret = cdx_mcdi_rpc(cdx, MC_CMD_CDX_BUS_UP, inbuf, sizeof(inbuf),
+ NULL, 0, NULL);
+
+ return ret;
+}
+
+int cdx_mcdi_bus_disable(struct cdx_mcdi *cdx, u8 bus_num)
+{
+ MCDI_DECLARE_BUF(inbuf, MC_CMD_CDX_BUS_DOWN_IN_LEN);
+ int ret;
+
+ MCDI_SET_DWORD(inbuf, CDX_BUS_DOWN_IN_BUS, bus_num);
+ ret = cdx_mcdi_rpc(cdx, MC_CMD_CDX_BUS_DOWN, inbuf, sizeof(inbuf),
+ NULL, 0, NULL);
+
+ return ret;
+}
+
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);
diff --git a/drivers/cdx/controller/mcdi_functions.h b/drivers/cdx/controller/mcdi_functions.h
index 7440ace5539a..28973d5ec3ab 100644
--- a/drivers/cdx/controller/mcdi_functions.h
+++ b/drivers/cdx/controller/mcdi_functions.h
@@ -47,6 +47,24 @@ int cdx_mcdi_get_dev_config(struct cdx_mcdi *cdx,
u8 bus_num, u8 dev_num,
struct cdx_dev_params *dev_params);
+/**
+ * cdx_mcdi_bus_enable - Enable CDX bus represented by bus_num
+ * @cdx: pointer to MCDI interface.
+ * @bus_num: Bus number.
+ *
+ * Return: 0 on success, <0 on failure
+ */
+int cdx_mcdi_bus_enable(struct cdx_mcdi *cdx, u8 bus_num);
+
+/**
+ * cdx_mcdi_bus_disable - Disable CDX bus represented by bus_num
+ * @cdx: pointer to MCDI interface.
+ * @bus_num: Bus number.
+ *
+ * Return: 0 on success, <0 on failure
+ */
+int cdx_mcdi_bus_disable(struct cdx_mcdi *cdx, u8 bus_num);
+
/**
* cdx_mcdi_reset_device - Reset cdx device represented by bus_num:dev_num
* @cdx: pointer to MCDI interface.
diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
index b5e4b7e05666..18e95076d1d5 100644
--- a/include/linux/cdx/cdx_bus.h
+++ b/include/linux/cdx/cdx_bus.h
@@ -28,6 +28,10 @@ struct cdx_device_config {
u8 type;
};
+typedef int (*cdx_bus_enable_cb)(struct cdx_controller *cdx, u8 bus_num);
+
+typedef int (*cdx_bus_disable_cb)(struct cdx_controller *cdx, u8 bus_num);
+
typedef int (*cdx_scan_cb)(struct cdx_controller *cdx);
typedef int (*cdx_dev_configure_cb)(struct cdx_controller *cdx,
@@ -49,11 +53,15 @@ typedef int (*cdx_dev_configure_cb)(struct cdx_controller *cdx,
/**
* struct cdx_ops - Callbacks supported by CDX controller.
+ * @bus_enable: enable bus on the controller
+ * @bus_disable: disable bus on the 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_bus_enable_cb bus_enable;
+ cdx_bus_disable_cb bus_disable;
cdx_scan_cb scan;
cdx_dev_configure_cb dev_configure;
};
@@ -89,6 +97,7 @@ struct cdx_controller {
* @flags: CDX device flags
* @req_id: Requestor ID associated with CDX device
* @is_bus: Is this bus device
+ * @enabled: is this bus enabled
* @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.
@@ -106,6 +115,7 @@ struct cdx_device {
u16 flags;
u32 req_id;
bool is_bus;
+ bool enabled;
const char *driver_override;
};
--
2.34.1